Re: [PATCH v2 04/12] virtio-iommu: Rename reserved_regions into prop_resv_regions

[Cédric Le Goater]

On 9/13/23 10:01, Eric Auger wrote:
> Rename VirtIOIOMMU (nb_)reserved_regions fields with the "prop_" prefix
> to highlight those fields are set through a property, at machine level.
> They are IOMMU wide.
>
> A subsequent patch will introduce per IOMMUDevice reserved regions
> that will include both those IOMMU wide property reserved
> regions plus, sometimes, host reserved regions, if the device is
> backed by a host device protected by a physical IOMMU. Also change
> nb_ prefix by nr_.


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>   include/hw/virtio/virtio-iommu.h |  4 ++--
>   hw/virtio/virtio-iommu-pci.c     |  8 ++++----
>   hw/virtio/virtio-iommu.c         | 15 ++++++++-------
>   3 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
> index a93fc5383e..eea4564782 100644
> --- a/include/hw/virtio/virtio-iommu.h
> +++ b/include/hw/virtio/virtio-iommu.h
> @@ -55,8 +55,8 @@ struct VirtIOIOMMU {
>       GHashTable *as_by_busptr;
>       IOMMUPciBus *iommu_pcibus_by_bus_num[PCI_BUS_MAX];
>       PCIBus *primary_bus;
> -    ReservedRegion *reserved_regions;
> -    uint32_t nb_reserved_regions;
> +    ReservedRegion *prop_resv_regions;
> +    uint32_t nr_prop_resv_regions;
>       GTree *domains;
>       QemuRecMutex mutex;
>       GTree *endpoints;
> diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
> index 7ef2f9dcdb..9459fbf6ed 100644
> --- a/hw/virtio/virtio-iommu-pci.c
> +++ b/hw/virtio/virtio-iommu-pci.c
> @@ -37,7 +37,7 @@ 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,
> +                      vdev.nr_prop_resv_regions, vdev.prop_resv_regions,
>                         qdev_prop_reserved_region, ReservedRegion),
>       DEFINE_PROP_END_OF_LIST(),
>   };
> @@ -54,9 +54,9 @@ static void virtio_iommu_pci_realize(VirtIOPCIProxy 
> *vpci_dev, Error **errp)
>                            "for the virtio-iommu-pci device");
>           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) {
> +    for (int i = 0; i < s->nr_prop_resv_regions; i++) {
> +        if (s->prop_resv_regions[i].type != VIRTIO_IOMMU_RESV_MEM_T_RESERVED &&
> +            s->prop_resv_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 0 and 1\n");
>               return;
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index e5e46e1b55..979cdb5648 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -631,22 +631,23 @@ static ssize_t 
> virtio_iommu_fill_resv_mem_prop(VirtIOIOMMU *s, uint32_t ep,
>       size_t size = sizeof(prop), length = size - sizeof(prop.head), total;
>       int i;
>   
> -    total = size * s->nb_reserved_regions;
> +    total = size * s->nr_prop_resv_regions;
>   
>       if (total > free) {
>           return -ENOSPC;
>       }
>   
> -    for (i = 0; i < s->nb_reserved_regions; i++) {
> -        unsigned subtype = s->reserved_regions[i].type;
> +    for (i = 0; i < s->nr_prop_resv_regions; i++) {
> +        unsigned subtype = s->prop_resv_regions[i].type;
> +        Range *range = &s->prop_resv_regions[i].range;
>   
>           assert(subtype == VIRTIO_IOMMU_RESV_MEM_T_RESERVED ||
>                  subtype == VIRTIO_IOMMU_RESV_MEM_T_MSI);
>           prop.head.type = cpu_to_le16(VIRTIO_IOMMU_PROBE_T_RESV_MEM);
>           prop.head.length = cpu_to_le16(length);
>           prop.subtype = subtype;
> -        prop.start = cpu_to_le64(range_lob(&s->reserved_regions[i].range));
> -        prop.end = cpu_to_le64(range_upb(&s->reserved_regions[i].range));
> +        prop.start = cpu_to_le64(range_lob(range));
> +        prop.end = cpu_to_le64(range_upb(range));
>   
>           memcpy(buf, &prop, size);
>   
> @@ -894,8 +895,8 @@ static IOMMUTLBEntry virtio_iommu_translate(IOMMUMemoryRegion *mr, hwaddr addr,
>           goto unlock;
>       }
>   
> -    for (i = 0; i < s->nb_reserved_regions; i++) {
> -        ReservedRegion *reg = &s->reserved_regions[i];
> +    for (i = 0; i < s->nr_prop_resv_regions; i++) {
> +        ReservedRegion *reg = &s->prop_resv_regions[i];
>   
>           if (range_contains(&reg->range, addr)) {
>               switch (reg->type) {