On Mon, Apr 16, 2012 at 12:12:52PM -0400, Jason Baron wrote:
> 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@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;
> > -
> > - /* 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,
I think it won't since all other bits are marked as emulated now.
> 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
--
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
