On Mon, 2012-04-16 at 18:06 +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@xxxxxxxxxx>
>
> ---
> 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;
This doesn't look right ^^^, we'll never make use of host side INTx
disable support that way. Thanks,
Alex
> -
> - /* 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);
> }
>
> static int assigned_initfn(struct PCIDevice *pci_dev)
> @@ -1658,7 +1645,6 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
> * device initialization.
> */
> assigned_dev_emulate_config_read(dev, 0, PCI_CONFIG_SPACE_SIZE);
> - assigned_dev_direct_config_read(dev, PCI_COMMAND, 2);
> assigned_dev_direct_config_read(dev, PCI_STATUS, 2);
> assigned_dev_direct_config_read(dev, PCI_REVISION_ID, 1);
> assigned_dev_direct_config_read(dev, PCI_CLASS_PROG, 3);
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
