Hi Alex,
On 1/24/24 15:15, Alex Williamson wrote:
> On Wed, 24 Jan 2024 14:57:41 +0100
> Eric Auger <eric.au...@redhat.com> wrote:
>
>> Hi Alex,
>>
>> On 1/24/24 14:37, Alex Williamson wrote:
>>> On Wed, 24 Jan 2024 14:14:19 +0100
>>> Eric Auger <eric.au...@redhat.com> wrote:
>>>
>>>> Hi Alex,
>>>>
>>>> On 1/24/24 00:51, Alex Williamson wrote:
>>>>> On Tue, 23 Jan 2024 19:15:55 +0100
>>>>> Eric Auger <eric.au...@redhat.com> wrote:
>>>>>
>>>>>> aw-bits is a new option that allows to set the bit width of
>>>>>> the input address range. This value will be used as a default for
>>>>>> the device config input_range.end. By default it is set to 64 bits
>>>>>> which is the current value.
>>>>>>
>>>>>> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>>>>>> ---
>>>>>> include/hw/virtio/virtio-iommu.h | 1 +
>>>>>> hw/virtio/virtio-iommu.c | 4 +++-
>>>>>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/include/hw/virtio/virtio-iommu.h
>>>>>> b/include/hw/virtio/virtio-iommu.h
>>>>>> index 781ebaea8f..5fbe4677c2 100644
>>>>>> --- a/include/hw/virtio/virtio-iommu.h
>>>>>> +++ b/include/hw/virtio/virtio-iommu.h
>>>>>> @@ -66,6 +66,7 @@ struct VirtIOIOMMU {
>>>>>> bool boot_bypass;
>>>>>> Notifier machine_done;
>>>>>> bool granule_frozen;
>>>>>> + uint8_t aw_bits;
>>>>>> };
>>>>>>
>>>>>> #endif
>>>>>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>>>>>> index ec2ba11d1d..e7f299e0c6 100644
>>>>>> --- a/hw/virtio/virtio-iommu.c
>>>>>> +++ b/hw/virtio/virtio-iommu.c
>>>>>> @@ -1314,7 +1314,8 @@ static void
>>>>>> virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>>>>>> */
>>>>>> s->config.bypass = s->boot_bypass;
>>>>>> s->config.page_size_mask = qemu_real_host_page_mask();
>>>>>> - s->config.input_range.end = UINT64_MAX;
>>>>>> + s->config.input_range.end =
>>>>>> + s->aw_bits == 64 ? UINT64_MAX : BIT_ULL(s->aw_bits) - 1;
>>>>> What happens when someone sets aw_bits = 1? There are a whole bunch of
>>>>> impractical values here ripe for annoying bug reports. vtd_realize()
>>>>> returns an Error for any values other than 39 or 48. We might pick an
>>>>> arbitrary lower bound (39?) or some other more creative solution here
>>>>> to avoid those silly issues in our future. Thanks,
>>>> You're right. I can check the input value. This needs to be dependent on
>>>> the machine though but this should be feasable.
>>>> Then I would allow 39 and 48 for q35 and 64 only on ARM.
>>> AFAIK AMD-Vi supports 64-bit address space. Without querying the host
>>> there's no way to place an accurate limit below 64-bit. Thanks,
>> Hum this means I would need to look at
>> /sys/class/iommu/<iommu>/amd-iommu/ or /sys/class/iommu/dmar* to
>> discriminate between AMD IOMMU and INTEL IOMMU physical IOMMU. Would
>> that be acceptable?
> I'm not necessarily suggesting that we look at the host, I'm mostly
> just stating that enforcing 39/48 bits on non-ARM is incorrect for a
> large portion of the non-ARM world too. There might even be some
> interesting use cases for a 32-bit IOVA space, so maybe just set
> defaults tuned for compatibility and accept anything between 32- and
> 64-bits. Thanks,
Yup that would be even simpler. Thank you for the clarification.
I will add that
Eric
>
> Alex
>
>>>>>> s->config.domain_range.end = UINT32_MAX;
>>>>>> s->config.probe_size = VIOMMU_PROBE_SIZE;
>>>>>>
>>>>>> @@ -1525,6 +1526,7 @@ static Property virtio_iommu_properties[] = {
>>>>>> DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
>>>>>> TYPE_PCI_BUS, PCIBus *),
>>>>>> DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
>>>>>> + DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
>>>>>> DEFINE_PROP_END_OF_LIST(),
>>>>>> };
>>>>>>
