Re: [PATCH v2] kvm: Disable MSI/MSI-X in assigned device reset path

[Jason Baron <jbaron@redhat.com>]

On Mon, Apr 16, 2012 at 06:06:40PM +0300, Michael S. Tsirkin wrote:
> On Mon, Apr 16, 2012 at 08:03:17AM -0600, Alex Williamson wrote:
> > The discussion on this patch seems to have fizzled, with no clear short
> > term solution.
> 
> I think we are in concensus, it's just that there are
> multiple bugs still left to fix.
> 
> First, we need to prevent guest from touching command
> register except for the bus master bit. Something like
> the below? Compiled only.
> 
> device-assignment: don't touch pci command register
> 
> Real command register is under kernel control:
> it includes bits for triggering SERR, marking
> BARs as invalid and such which are under host
> kernel control. Don't touch any except bus master
> which is ok to put under guest control.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> ---
> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> index 89823f1..9ebce49 100644
> --- a/hw/device-assignment.c
> +++ b/hw/device-assignment.c
> @@ -501,7 +501,6 @@ static int get_real_device(AssignedDevice *pci_dev, uint16_t r_seg,
>      FILE *f;
>      unsigned long long start, end, size, flags;
>      uint16_t id;
> -    struct stat statbuf;
>      PCIRegion *rp;
>      PCIDevRegions *dev = &pci_dev->real_device;
>  
> @@ -610,12 +609,8 @@ again:
>      pci_dev->dev.config[2] = id & 0xff;
>      pci_dev->dev.config[3] = (id & 0xff00) >> 8;
>  
> -    /* dealing with virtual function device */
> -    snprintf(name, sizeof(name), "%sphysfn/", dir);
> -    if (!stat(name, &statbuf)) {
> -        /* always provide the written value on readout */
> -        assigned_dev_emulate_config_read(pci_dev, PCI_COMMAND, 2);
> -    }
> +    /* Pass bus master writes to device. */
> +    pci_dev->emulate_config_write[PCI_COMMAND] &= ~PCI_COMMAND_MASTER;
>  
>      dev->region_number = r;
>      return 0;
> @@ -782,14 +777,6 @@ static int assign_device(AssignedDevice *dev)
>                  "cause host memory corruption if the device issues DMA write "
>                  "requests!\n");
>      }
> -    if (dev->features & ASSIGNED_DEVICE_SHARE_INTX_MASK &&
> -        kvm_has_intx_set_mask()) {
> -        assigned_dev_data.flags |= KVM_DEV_ASSIGN_PCI_2_3;
> -
> -        /* hide host-side INTx masking from the guest */
> -        dev->emulate_config_read[PCI_COMMAND + 1] |=
> -            PCI_COMMAND_INTX_DISABLE >> 8;
> -    }
>  
>      r = kvm_assign_pci_device(kvm_state, &assigned_dev_data);
>      if (r < 0) {
> @@ -1631,10 +1618,10 @@ static void reset_assigned_device(DeviceState *dev)
>      }
>  
>      /*
> -     * When a 0 is written to the command register, the device is logically
> +     * When a 0 is written to the bus master register, the device is logically
>       * disconnected from the PCI bus. This avoids further DMA transfers.
>       */
> -    assigned_dev_pci_write_config(pci_dev, PCI_COMMAND, 0, 2);
> +    assigned_dev_pci_write_config(pci_dev, PCI_COMMAND, 0, 1);
>  }

This is still going to disable mmio, is the intent to just clear the bus
master bit, ie bit 2? Or is this patch meant to be in addition to the
one Alex posted?

Thanks,

-Jason