Retrieve the Host IOMMU Device page size mask when this latter is set.
This allows to get the information much sooner than when relying on
IOMMU MR set_page_size_mask() call, whcih happens when the IOMMU MR
gets enabled. We introduce check_page_size_mask() helper whose code
is inherited from current virtio_iommu_set_page_size_mask()
implementation. This callback will be removed in a subsequent patch.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
hw/virtio/virtio-iommu.c | 55 ++++++++++++++++++++++++++++++++++++++--
hw/virtio/trace-events | 1 +
2 files changed, 54 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index b8f75d2b1a..631589735a 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -598,9 +598,39 @@ out:
return ret;
}
+static bool check_page_size_mask(VirtIOIOMMU *viommu, uint64_t new_mask,
+ Error **errp)
+{
+ uint64_t cur_mask = viommu->config.page_size_mask;
+
+ if ((cur_mask & new_mask) == 0) {
+ error_setg(errp, "virtio-iommu reports a page size mask 0x%"PRIx64
+ " incompatible with currently supported mask 0x%"PRIx64,
+ new_mask, cur_mask);
+ return false;
+ }
+ /*
+ * Once the granule is frozen we can't change the mask anymore. If by
+ * chance the hotplugged device supports the same granule, we can still
+ * accept it.
+ */
+ if (viommu->granule_frozen) {
+ int cur_granule = ctz64(cur_mask);
+
+ if (!(BIT_ULL(cur_granule) & new_mask)) {
+ error_setg(errp,
+ "virtio-iommu does not support frozen granule 0x%llx",
+ BIT_ULL(cur_granule));
+ return false;
+ }
+ }
+ return true;
+}
+
static bool virtio_iommu_set_iommu_device(PCIBus *bus, void *opaque, int devfn,
HostIOMMUDevice *hiod, Error **errp)
{
+ ERRP_GUARD();
VirtIOIOMMU *viommu = opaque;
HostIOMMUDeviceClass *hiodc = HOST_IOMMU_DEVICE_GET_CLASS(hiod);
struct hiod_key *new_key;
@@ -623,8 +653,26 @@ static bool virtio_iommu_set_iommu_device(PCIBus *bus, void *opaque, int devfn,
hiod->aliased_devfn,
host_iova_ranges, errp);
if (ret) {
- g_list_free_full(host_iova_ranges, g_free);
- return false;
+ goto error;
+ }
+ }
+ if (hiodc->get_page_size_mask) {
+ uint64_t new_mask = hiodc->get_page_size_mask(hiod);
+
+ if (check_page_size_mask(viommu, new_mask, errp)) {
+ /*
+ * The default mask depends on the "granule" property. For example,
+ * with 4k granule, it is -(4 * KiB). When an assigned device has
+ * page size restrictions due to the hardware IOMMU configuration,
+ * apply this restriction to the mask.
+ */
+ trace_virtio_iommu_update_page_size_mask(hiod->name,
+ viommu->config.page_size_mask,
+ new_mask);
+ viommu->config.page_size_mask &= new_mask;
+ } else {
+ error_prepend(errp, "%s: ", hiod->name);
+ goto error;
}
}
@@ -637,6 +685,9 @@ static bool virtio_iommu_set_iommu_device(PCIBus *bus, void *opaque, int devfn,
g_list_free_full(host_iova_ranges, g_free);
return true;
+error:
+ g_list_free_full(host_iova_ranges, g_free);
+ return false;
}
static void
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 3cf84e04a7..599d855ff6 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -132,6 +132,7 @@ virtio_iommu_notify_map(const char *name, uint64_t virt_start, uint64_t virt_end
virtio_iommu_notify_unmap(const char *name, uint64_t virt_start, uint64_t virt_end) "mr=%s virt_start=0x%"PRIx64" virt_end=0x%"PRIx64
virtio_iommu_remap(const char *name, uint64_t virt_start, uint64_t virt_end, uint64_t phys_start) "mr=%s virt_start=0x%"PRIx64" virt_end=0x%"PRIx64" phys_start=0x%"PRIx64
virtio_iommu_set_page_size_mask(const char *name, uint64_t old, uint64_t new) "mr=%s old_mask=0x%"PRIx64" new_mask=0x%"PRIx64
+virtio_iommu_update_page_size_mask(const char *name, uint64_t old, uint64_t new) "host iommu device=%s old_mask=0x%"PRIx64" new_mask=0x%"PRIx64
virtio_iommu_notify_flag_add(const char *name) "add notifier to mr %s"
virtio_iommu_notify_flag_del(const char *name) "del notifier from mr %s"
virtio_iommu_switch_address_space(uint8_t bus, uint8_t slot, uint8_t fn, bool on) "Device %02x:%02x.%x switching address space (iommu enabled=%d)"
--
2.41.0Hi Eric,
>-----Original Message-----
>From: Eric Auger <eric.auger@redhat.com>
>Subject: [PATCH 5/7] virtio-iommu : Retrieve page size mask on
>virtio_iommu_set_iommu_device()
>
>Retrieve the Host IOMMU Device page size mask when this latter is set.
>This allows to get the information much sooner than when relying on
>IOMMU MR set_page_size_mask() call, whcih happens when the IOMMU
>MR
>gets enabled. We introduce check_page_size_mask() helper whose code
>is inherited from current virtio_iommu_set_page_size_mask()
>implementation. This callback will be removed in a subsequent patch.
>
>Signed-off-by: Eric Auger <eric.auger@redhat.com>
>---
> hw/virtio/virtio-iommu.c | 55
>++++++++++++++++++++++++++++++++++++++--
> hw/virtio/trace-events | 1 +
> 2 files changed, 54 insertions(+), 2 deletions(-)
>
>diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>index b8f75d2b1a..631589735a 100644
>--- a/hw/virtio/virtio-iommu.c
>+++ b/hw/virtio/virtio-iommu.c
>@@ -598,9 +598,39 @@ out:
> return ret;
> }
>
>+static bool check_page_size_mask(VirtIOIOMMU *viommu, uint64_t
>new_mask,
>+ Error **errp)
>+{
>+ uint64_t cur_mask = viommu->config.page_size_mask;
>+
>+ if ((cur_mask & new_mask) == 0) {
>+ error_setg(errp, "virtio-iommu reports a page size mask 0x%"PRIx64
>+ " incompatible with currently supported mask 0x%"PRIx64,
>+ new_mask, cur_mask);
>+ return false;
>+ }
>+ /*
>+ * Once the granule is frozen we can't change the mask anymore. If by
>+ * chance the hotplugged device supports the same granule, we can still
>+ * accept it.
>+ */
>+ if (viommu->granule_frozen) {
>+ int cur_granule = ctz64(cur_mask);
>+
>+ if (!(BIT_ULL(cur_granule) & new_mask)) {
>+ error_setg(errp,
>+ "virtio-iommu does not support frozen granule 0x%llx",
>+ BIT_ULL(cur_granule));
>+ return false;
>+ }
>+ }
>+ return true;
>+}
>+
> static bool virtio_iommu_set_iommu_device(PCIBus *bus, void *opaque,
>int devfn,
> HostIOMMUDevice *hiod, Error **errp)
> {
>+ ERRP_GUARD();
> VirtIOIOMMU *viommu = opaque;
> HostIOMMUDeviceClass *hiodc =
>HOST_IOMMU_DEVICE_GET_CLASS(hiod);
> struct hiod_key *new_key;
>@@ -623,8 +653,26 @@ static bool
>virtio_iommu_set_iommu_device(PCIBus *bus, void *opaque, int devfn,
> hiod->aliased_devfn,
> host_iova_ranges, errp);
> if (ret) {
>- g_list_free_full(host_iova_ranges, g_free);
>- return false;
>+ goto error;
>+ }
>+ }
>+ if (hiodc->get_page_size_mask) {
>+ uint64_t new_mask = hiodc->get_page_size_mask(hiod);
>+
>+ if (check_page_size_mask(viommu, new_mask, errp)) {
>+ /*
>+ * The default mask depends on the "granule" property. For example,
>+ * with 4k granule, it is -(4 * KiB). When an assigned device has
>+ * page size restrictions due to the hardware IOMMU configuration,
>+ * apply this restriction to the mask.
>+ */
>+ trace_virtio_iommu_update_page_size_mask(hiod->name,
>+ viommu->config.page_size_mask,
>+ new_mask);
>+ viommu->config.page_size_mask &= new_mask;
This is a bit different from original logic, it may update page_size_mask after frozen.
Will that make issue?
Except this question, for all other patches,
Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Thanks
Zhenzhong
>+ } else {
>+ error_prepend(errp, "%s: ", hiod->name);
>+ goto error;
> }
> }
>
>@@ -637,6 +685,9 @@ static bool
>virtio_iommu_set_iommu_device(PCIBus *bus, void *opaque, int devfn,
> g_list_free_full(host_iova_ranges, g_free);
>
> return true;
>+error:
>+ g_list_free_full(host_iova_ranges, g_free);
>+ return false;
> }
>
> static void
>diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
>index 3cf84e04a7..599d855ff6 100644
>--- a/hw/virtio/trace-events
>+++ b/hw/virtio/trace-events
>@@ -132,6 +132,7 @@ virtio_iommu_notify_map(const char *name,
>uint64_t virt_start, uint64_t virt_end
> virtio_iommu_notify_unmap(const char *name, uint64_t virt_start,
>uint64_t virt_end) "mr=%s virt_start=0x%"PRIx64" virt_end=0x%"PRIx64
> virtio_iommu_remap(const char *name, uint64_t virt_start, uint64_t
>virt_end, uint64_t phys_start) "mr=%s virt_start=0x%"PRIx64"
>virt_end=0x%"PRIx64" phys_start=0x%"PRIx64
> virtio_iommu_set_page_size_mask(const char *name, uint64_t old,
>uint64_t new) "mr=%s old_mask=0x%"PRIx64" new_mask=0x%"PRIx64
>+virtio_iommu_update_page_size_mask(const char *name, uint64_t old,
>uint64_t new) "host iommu device=%s old_mask=0x%"PRIx64"
>new_mask=0x%"PRIx64
> virtio_iommu_notify_flag_add(const char *name) "add notifier to mr %s"
> virtio_iommu_notify_flag_del(const char *name) "del notifier from mr %s"
> virtio_iommu_switch_address_space(uint8_t bus, uint8_t slot, uint8_t fn,
>bool on) "Device %02x:%02x.%x switching address space (iommu
>enabled=%d)"
>--
>2.41.0
Hi Zhenhong,
On 6/27/24 13:32, Duan, Zhenzhong wrote:
> Hi Eric,
>
>> -----Original Message-----
>> From: Eric Auger <eric.auger@redhat.com>
>> Subject: [PATCH 5/7] virtio-iommu : Retrieve page size mask on
>> virtio_iommu_set_iommu_device()
>>
>> Retrieve the Host IOMMU Device page size mask when this latter is set.
>> This allows to get the information much sooner than when relying on
>> IOMMU MR set_page_size_mask() call, whcih happens when the IOMMU
>> MR
>> gets enabled. We introduce check_page_size_mask() helper whose code
>> is inherited from current virtio_iommu_set_page_size_mask()
>> implementation. This callback will be removed in a subsequent patch.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>> hw/virtio/virtio-iommu.c | 55
>> ++++++++++++++++++++++++++++++++++++++--
>> hw/virtio/trace-events | 1 +
>> 2 files changed, 54 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>> index b8f75d2b1a..631589735a 100644
>> --- a/hw/virtio/virtio-iommu.c
>> +++ b/hw/virtio/virtio-iommu.c
>> @@ -598,9 +598,39 @@ out:
>> return ret;
>> }
>>
>> +static bool check_page_size_mask(VirtIOIOMMU *viommu, uint64_t
>> new_mask,
>> + Error **errp)
>> +{
>> + uint64_t cur_mask = viommu->config.page_size_mask;
>> +
>> + if ((cur_mask & new_mask) == 0) {
>> + error_setg(errp, "virtio-iommu reports a page size mask 0x%"PRIx64
>> + " incompatible with currently supported mask 0x%"PRIx64,
>> + new_mask, cur_mask);
>> + return false;
>> + }
>> + /*
>> + * Once the granule is frozen we can't change the mask anymore. If by
>> + * chance the hotplugged device supports the same granule, we can still
>> + * accept it.
>> + */
>> + if (viommu->granule_frozen) {
>> + int cur_granule = ctz64(cur_mask);
>> +
>> + if (!(BIT_ULL(cur_granule) & new_mask)) {
>> + error_setg(errp,
>> + "virtio-iommu does not support frozen granule 0x%llx",
>> + BIT_ULL(cur_granule));
>> + return false;
>> + }
>> + }
>> + return true;
>> +}
>> +
>> static bool virtio_iommu_set_iommu_device(PCIBus *bus, void *opaque,
>> int devfn,
>> HostIOMMUDevice *hiod, Error **errp)
>> {
>> + ERRP_GUARD();
>> VirtIOIOMMU *viommu = opaque;
>> HostIOMMUDeviceClass *hiodc =
>> HOST_IOMMU_DEVICE_GET_CLASS(hiod);
>> struct hiod_key *new_key;
>> @@ -623,8 +653,26 @@ static bool
>> virtio_iommu_set_iommu_device(PCIBus *bus, void *opaque, int devfn,
>> hiod->aliased_devfn,
>> host_iova_ranges, errp);
>> if (ret) {
>> - g_list_free_full(host_iova_ranges, g_free);
>> - return false;
>> + goto error;
>> + }
>> + }
>> + if (hiodc->get_page_size_mask) {
>> + uint64_t new_mask = hiodc->get_page_size_mask(hiod);
>> +
>> + if (check_page_size_mask(viommu, new_mask, errp)) {
>> + /*
>> + * The default mask depends on the "granule" property. For example,
>> + * with 4k granule, it is -(4 * KiB). When an assigned device has
>> + * page size restrictions due to the hardware IOMMU configuration,
>> + * apply this restriction to the mask.
>> + */
>> + trace_virtio_iommu_update_page_size_mask(hiod->name,
>> + viommu->config.page_size_mask,
>> + new_mask);
>> + viommu->config.page_size_mask &= new_mask;
> This is a bit different from original logic, it may update page_size_mask after frozen.
> Will that make issue?
You're fully right. I need to replace
viommu->config.page_size_mask &= new_mask;
by
if (s->granule_frozen) {
viommu->config.page_size_mask &= new_mask;
}
> Except this question, for all other patches,
>
> Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Thanks!
Eric
>
> Thanks
> Zhenzhong
>
>> + } else {
>> + error_prepend(errp, "%s: ", hiod->name);
>> + goto error;
>> }
>> }
>>
>> @@ -637,6 +685,9 @@ static bool
>> virtio_iommu_set_iommu_device(PCIBus *bus, void *opaque, int devfn,
>> g_list_free_full(host_iova_ranges, g_free);
>>
>> return true;
>> +error:
>> + g_list_free_full(host_iova_ranges, g_free);
>> + return false;
>> }
>>
>> static void
>> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
>> index 3cf84e04a7..599d855ff6 100644
>> --- a/hw/virtio/trace-events
>> +++ b/hw/virtio/trace-events
>> @@ -132,6 +132,7 @@ virtio_iommu_notify_map(const char *name,
>> uint64_t virt_start, uint64_t virt_end
>> virtio_iommu_notify_unmap(const char *name, uint64_t virt_start,
>> uint64_t virt_end) "mr=%s virt_start=0x%"PRIx64" virt_end=0x%"PRIx64
>> virtio_iommu_remap(const char *name, uint64_t virt_start, uint64_t
>> virt_end, uint64_t phys_start) "mr=%s virt_start=0x%"PRIx64"
>> virt_end=0x%"PRIx64" phys_start=0x%"PRIx64
>> virtio_iommu_set_page_size_mask(const char *name, uint64_t old,
>> uint64_t new) "mr=%s old_mask=0x%"PRIx64" new_mask=0x%"PRIx64
>> +virtio_iommu_update_page_size_mask(const char *name, uint64_t old,
>> uint64_t new) "host iommu device=%s old_mask=0x%"PRIx64"
>> new_mask=0x%"PRIx64
>> virtio_iommu_notify_flag_add(const char *name) "add notifier to mr %s"
>> virtio_iommu_notify_flag_del(const char *name) "del notifier from mr %s"
>> virtio_iommu_switch_address_space(uint8_t bus, uint8_t slot, uint8_t fn,
>> bool on) "Device %02x:%02x.%x switching address space (iommu
>> enabled=%d)"
>> --
>> 2.41.0
© 2016 - 2024 Red Hat, Inc.