On 2012-04-17 13:24, Jan Kiszka wrote: > On 2012-04-16 21:53, Michael S. Tsirkin wrote: >> 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 and intx >> mask which kvm interrupt sharing machinery >> explicitly allows. >> >> Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx> >> >> --- >> Compiled only, don't have a setup for assignment >> testing ATM. Is anyone interested enough to test >> and report? >> >> Changes from v1: >> - fix intx mask handling >> >> hw/device-assignment.c | 24 ++++++++---------------- >> 1 files changed, 8 insertions(+), 16 deletions(-) >> >> diff --git a/hw/device-assignment.c b/hw/device-assignment.c >> index 89823f1..6cdcc17 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,9 @@ 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; >> + pci_dev->emulate_config_write[PCI_COMMAND + 1] &= ~(PCI_COMMAND_INTX_DISABLE >> 8); > > Comment doesn't fully match the code. And I bet you aren't using > checkpatch... ;) > >> >> dev->region_number = r; >> return 0; >> @@ -782,13 +778,10 @@ 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; >> + if (dev->features & ASSIGNED_DEVICE_SHARE_INTX_MASK && >> + kvm_has_intx_set_mask()) { >> + assigned_dev_data.flags |= KVM_DEV_ASSIGN_PCI_2_3; >> } >> >> r = kvm_assign_pci_device(kvm_state, &assigned_dev_data); >> @@ -1631,10 +1624,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 +1651,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); > > Patch looks otherwise fine to me. Err, I mean v3 on which I actually wanted to comment. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux -- 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