On Mon, Apr 16, 2012 at 01:07:56PM -0600, Alex Williamson wrote:
> 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
Right.
--
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
