[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 2/2] VFIO: Clear stale MSIx table during EEH
From: |
Gavin Shan |
Subject: |
Re: [Qemu-devel] [RFC PATCH 2/2] VFIO: Clear stale MSIx table during EEH reset |
Date: |
Wed, 27 Aug 2014 23:15:41 +1000 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Tue, Aug 26, 2014 at 02:25:47PM -0600, Alex Williamson wrote:
>On Wed, 2014-08-20 at 19:52 +1000, Gavin Shan wrote:
>> The PCI device MSIx table is cleaned out in hardware after EEH PE
>> reset. However, we still hold the stale MSIx entries in QEMU, which
>> should be cleared accordingly. Otherwise, we will run into another
>> (recursive) EEH error and the PCI devices contained in the PE have
>> to be offlined exceptionally.
>>
>> The patch clears stale MSIx table before EEH PE reset so that MSIx
>> table could be restored properly after EEH PE reset.
>>
>> Signed-off-by: Gavin Shan <address@hidden>
>> ---
>> hw/misc/vfio.c | 24 ++++++++++++++++++++++++
>> 1 file changed, 24 insertions(+)
>>
>> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
>> index 1a3e7eb..3cf7f02 100644
>> --- a/hw/misc/vfio.c
>> +++ b/hw/misc/vfio.c
>> @@ -4424,6 +4424,8 @@ int vfio_container_ioctl(AddressSpace *as, int32_t
>> groupid,
>> {
>> VFIOGroup *group;
>> VFIOContainer *container;
>> + VFIODevice *vdev;
>> + struct vfio_eeh_pe_op *arg;
>
>Define these within the scope of the case since they're not used outside
>of it.
>
Yes, I'll fix.
>> int ret = -1;
>>
>> group = vfio_get_group(groupid, as);
>> @@ -4442,7 +4444,29 @@ int vfio_container_ioctl(AddressSpace *as, int32_t
>> groupid,
>> switch (req) {
>> case VFIO_CHECK_EXTENSION:
>> case VFIO_IOMMU_SPAPR_TCE_GET_INFO:
>> + break;
>> case VFIO_EEH_PE_OP:
>> + arg = (struct vfio_eeh_pe_op *)param;
>> + switch (arg->op) {
>> + case VFIO_EEH_PE_RESET_HOT:
>> + case VFIO_EEH_PE_RESET_FUNDAMENTAL:
>> + /*
>> + * The MSIx table will be cleaned out by reset. We need
>> + * disable it so that it can be reenabled properly. Also,
>> + * the cached MSIx table should be cleared as it's not
>> + * reflecting the contents in hardware.
>> + */
>> + QLIST_FOREACH(vdev, &group->device_list, next) {
>> + if (msix_enabled(&vdev->pdev)) {
>> + vfio_disable_msix(vdev);
>> + }
>> +
>> + msix_reset(&vdev->pdev);
>> + }
>
>We already have vfio_disable_interrupts(), can't we use that (blindly)?
>Do we really need to call msix_reset()? If so, should
>vfio_disable_msix() call it?
>
Yes, vfio_disable_interrupts() would be better to be used here as it
can covers all types of interrupt (INTx/MSI/MSIx).
vfio_disable_interrupts() needn't clear MSIx vectors. If you prefer
calling msix_reset() in the function, I guess I have to add one more
parameter to vfio_disable_interrupts() to indicate if we need clear
MSI/MSIx vector:
static void vfio_disable_interrupts(VFIODevice *vdev, bool clr_vector)
>> +
>> + break;
>
>Extraneous break
>
Yep, I'll remove it.
>> + }
>> +
>> break;
>> default:
>> /* Return an error on unknown requests */
>
>Thanks,
>Alex
>
Thanks,
Gavin
[Qemu-devel] [RFC PATCH 1/2] VFIO: Drop vfio_container_do_ioctl(), Gavin Shan, 2014/08/20
Re: [Qemu-devel] [RFC PATCH 0/2] Fix MSIx lost after PE reset, Gavin Shan, 2014/08/25