Hi Clement,
>-----Original Message-----
>From: CLEMENT MATHIEU--DRIF <clement.mathieu--d...@eviden.com>
>Sent: Friday, May 24, 2024 9:57 PM
>To: Duan, Zhenzhong <zhenzhong.d...@intel.com>; qemu-
>de...@nongnu.org
>Cc: alex.william...@redhat.com; c...@redhat.com; eric.au...@redhat.com;
>m...@redhat.com; pet...@redhat.com; jasow...@redhat.com;
>j...@nvidia.com; nicol...@nvidia.com; joao.m.mart...@oracle.com; Tian,
>Kevin <kevin.t...@intel.com>; Liu, Yi L <yi.l....@intel.com>; Peng, Chao P
><chao.p.p...@intel.com>; Paolo Bonzini <pbonz...@redhat.com>; Richard
>Henderson <richard.hender...@linaro.org>; Eduardo Habkost
><edua...@habkost.net>; Marcel Apfelbaum <marcel.apfelb...@gmail.com>
>Subject: Re: [PATCH rfcv2 15/17] intel_iommu: Set default aw_bits to 48 in
>scalable modren mode
>
>Hi Zhenzhong
>
>On 22/05/2024 08:23, Zhenzhong Duan wrote:
>> Caution: External email. Do not open attachments or click links, unless this
>email comes from a known sender and you know the content is safe.
>>
>>
>> According to VTD spec, stage-1 page table could support 4-level and
>> 5-level paging.
>>
>> However, 5-level paging translation emulation is unsupported yet.
>> That means the only supported value for aw_bits is 48.
>>
>> So default aw_bits to 48 in scalable modern mode. In other cases,
>> it is still default to 39 for compatibility.
>>
>> Add a check to ensure user specified value is 48 in modern mode
>> for now.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>> ---
>> hw/i386/intel_iommu.c | 16 +++++++++++++++-
>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index e07daaba99..a4c241ea96 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -3748,7 +3748,7 @@ static Property vtd_properties[] = {
>> ON_OFF_AUTO_AUTO),
>> DEFINE_PROP_BOOL("x-buggy-eim", IntelIOMMUState, buggy_eim,
>false),
>> DEFINE_PROP_UINT8("aw-bits", IntelIOMMUState, aw_bits,
>> - VTD_HOST_ADDRESS_WIDTH),
>> + 0xff),
>you could define a constant for this invalid value
Sure, maybe VTD_HOST_ADDRESS_WIDTH_UNDEFINED?
Thanks
Zhenzhong
>> DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode,
>FALSE),
>> DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState,
>scalable_mode, FALSE),
>> DEFINE_PROP_BOOL("snoop-control", IntelIOMMUState,
>snoop_control, false),
>> @@ -4663,6 +4663,14 @@ static bool
>vtd_decide_config(IntelIOMMUState *s, Error **errp)
>> }
>> }
>>
>> + if (s->aw_bits == 0xff) {
>> + if (s->scalable_modern) {
>> + s->aw_bits = VTD_HOST_AW_48BIT;
>> + } else {
>> + s->aw_bits = VTD_HOST_AW_39BIT;
>> + }
>> + }
>> +
>> if ((s->aw_bits != VTD_HOST_AW_39BIT) &&
>> (s->aw_bits != VTD_HOST_AW_48BIT) &&
>> !s->scalable_modern) {
>> @@ -4671,6 +4679,12 @@ static bool
>vtd_decide_config(IntelIOMMUState *s, Error **errp)
>> return false;
>> }
>>
>> + if ((s->aw_bits != VTD_HOST_AW_48BIT) && s->scalable_modern) {
>> + error_setg(errp, "Supported values for aw-bits are: %d",
>specify 'in modern mode' in the message?
>> + VTD_HOST_AW_48BIT);
>> + return false;
>> + }
>> +
>> if (s->scalable_mode && !s->dma_drain) {
>> error_setg(errp, "Need to set dma_drain for scalable mode");
>> return false;
>> --
>> 2.34.1
>>
>#cmd
