Hi Markus, On 6/27/20 9:03 AM, Markus Armbruster wrote: > Eric Auger <eric.au...@redhat.com> writes: > >> The machine may need to pass reserved regions to the >> virtio-iommu-pci device (such as the MSI window on x86 >> or the MSI doorbells on ARM). >> >> So let's add an array of Interval properties. >> >> Note: if some reserved regions are already set by the >> machine code - which should be the case in general -, >> the length of the property array is already set and >> prevents the end-user from modifying them. For example, >> attempting to use: >> >> -device virtio-iommu-pci,\ >> len-reserved-regions=1,reserved-regions[0]=0xfee00000:0xfeefffff:1 >> >> would result in the following error message: >> >> qemu-system-aarch64: -device virtio-iommu-pci,addr=0xa, >> len-reserved-regions=1,reserved-regions[0]=0xfee00000:0xfeefffff:1: >> array size property len-reserved-regions may not be set more than once >> >> Otherwise, for example, adding two reserved regions is achieved >> using the following options: >> >> -device virtio-iommu-pci,addr=0xa,len-reserved-regions=2,\ >> reserved-regions[0]=0xfee00000:0xfeefffff:1,\ >> reserved-regions[1]=0x1000000:100ffff:1 >> >> Signed-off-by: Eric Auger <eric.au...@redhat.com> >> >> --- >> v5 -> v6: >> - check the type value >> - removed Jean's R-b >> >> v3 -> v4: >> - added examples in the commit message as suggested by Markus >> - added Jean's R-b >> --- >> hw/virtio/virtio-iommu-pci.c | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c >> index 632533abaf..9c9bda9490 100644 >> --- a/hw/virtio/virtio-iommu-pci.c >> +++ b/hw/virtio/virtio-iommu-pci.c >> @@ -33,6 +33,9 @@ struct VirtIOIOMMUPCI { >> >> static Property virtio_iommu_pci_properties[] = { >> DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0), >> + DEFINE_PROP_ARRAY("reserved-regions", VirtIOIOMMUPCI, >> + vdev.nb_reserved_regions, vdev.reserved_regions, >> + qdev_prop_reserved_region, ReservedRegion), >> DEFINE_PROP_END_OF_LIST(), >> }; >> >> @@ -40,6 +43,7 @@ static void virtio_iommu_pci_realize(VirtIOPCIProxy >> *vpci_dev, Error **errp) >> { >> VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(vpci_dev); >> DeviceState *vdev = DEVICE(&dev->vdev); >> + VirtIOIOMMU *s = VIRTIO_IOMMU(vdev); >> >> if (!qdev_get_machine_hotplug_handler(DEVICE(vpci_dev))) { >> MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); >> @@ -54,6 +58,15 @@ static void virtio_iommu_pci_realize(VirtIOPCIProxy >> *vpci_dev, Error **errp) >> "-no-acpi\n"); >> return; >> } >> + for (int i = 0; i < s->nb_reserved_regions; i++) { >> + if (s->reserved_regions[i].type != VIRTIO_IOMMU_RESV_MEM_T_RESERVED >> && >> + s->reserved_regions[i].type != VIRTIO_IOMMU_RESV_MEM_T_MSI) { >> + error_setg(errp, "reserved region %d has an invalid type", i); >> + error_append_hint(errp, "Valid values are " >> + "VIRTIO_IOMMU_RESV_MEM_T_RESERVED and " >> + "VIRTIO_IOMMU_RESV_MEM_T_SMI\n"); > > Actually, valid values are 0 and 1. Something like > > -device > virtio-iommu-pci,len-reserved-regions=1,reserved-regions[0]=0xfee00000:0xfeefffff:VIRTIO_IOMMU_RESV_MEM_T_SMI > > will be rejected as invalid.
> >> + } >> + } >> object_property_set_link(OBJECT(dev), >> OBJECT(pci_get_bus(&vpci_dev->pci_dev)), >> "primary-bus", errp); > > Sorry for having to torture you this way! Have a Nah In french we say "when we have no head we have legs ". Anyway there is a type s/SMI/MSI ... > Reviewed-by: Markus Armbruster <arm...@redhat.com> > with the error message touched up. Than you for your patience! Eric > >