Re: [Qemu-devel] [PATCH v1 2/2] intel-iommu: Extend address width to 48 bits

[Prasad Singamsetty <prasad.singamsetty@oracle.com>]

On 11/30/2017 10:56 AM, Michael S. Tsirkin wrote:
> On Thu, Nov 30, 2017 at 10:33:50AM -0800, Prasad Singamsetty wrote:
>>
>>
>> On 11/29/2017 7:25 PM, Peter Xu wrote:
>>> On Wed, Nov 29, 2017 at 01:05:22PM -0800, Prasad Singamsetty wrote:
>>>> Thanks Michael. Some comments below.
>>>>
>>>> On 11/28/2017 9:32 AM, Michael S. Tsirkin wrote:
>>>>> On Tue, Nov 14, 2017 at 06:13:50PM -0500, prasad.singamsetty@oracle.com wrote:
>>>>>> From: Prasad Singamsetty <prasad.singamsetty@oracle.com>
>>>>>>
>>>>>> The current implementation of Intel IOMMU code only supports 39 bits
>>>>>> iova address width. This patch provides a new parameter (x-aw-bits)
>>>>>> for intel-iommu to extend its address width to 48 bits but keeping the
>>>>>> default the same (39 bits). The reason for not changing the default
>>>>>> is to avoid potential compatibility problems
>>>>>
>>>>> You can change the default, just make it 39 for existing machine types.
>>>>
>>>> I think introducing a new machine type is not appropriate as this
>>>> is an implementation limitation for the existing machine type.
>>>> Currently q35 is the only machine type that supports intel-iommu.
>>>> And we want to retain the current default behavior for q35 to avoid
>>>> any new issues with live migration.
>>>
>>> I guess "existing machine type" means e.g. pc-q35-2.11 and older ones,
>>> rather than creating another machine type in parallel with q35.  So we
>>> can set 48 bits as default on upcoming pc-q35-2.12 machines, while
>>> keep the 39 bits on the old ones.
>>>
>>> Please refer to include/hw/compat.h.
>>
>> Thanks Peter, for the clarification and pointer to this. I am still
>> new to this but learning on how this works or how this is used in
>> use cases like Live Migration.
>>
>> Are you suggesting that we change the default to 48 bits in the
>> next release (2.12)?
>>
>> User need to specify an older machine type  (pc-q35-2.11 or older)
>> to get the old default value of 39 bits. This still requires the
>> patch I proposed to support compatibility for older releases
>> except the introduction of the new property (x-aw-bits).
> 
> Yes. If you see a reason for users to limit it to 39 bits,
> we can make it a supported property (not starting
> with x-). If it's only for live migration, we can
> use a non-supported property (with x-).
I think it is only for Live Migration case, we need to
support the old default value of 39 bits.

Do you see any need to keep a non-supported property?
It may be useful for developers.

> 
>>
>>>
>>>>
>>>>>
>>>>>> with live migration of
>>>>>> intel-iommu enabled QEMU guest. The only valid values for 'x-aw-bits'
>>>>>> parameter are 39 and 48.
>>>>>
>>>>> I'd rather make it a boolean then.
>>>>
>>>> Right. It seems Intel already has additional sizes supported so keeping
>>>> it as an integer seems better.
>>>
>>> Yes, considering that 5-level IOMMUs are coming (AFAIK).
>>>
>>
>> If we change the default value to 48 bits, I assume there is
>> no need for this property and user is expected to use an
>> older machine type based on the release to get the old
>> default. Is this correct?
>>
>> Thanks.
>> --Prasad
> 
> 
> No, users use an older machine type to get migration from old
> machine types. If someone might actually want 39 bit for
> some reason, we need it as a supported property.

OK. Other than Live Migration case I don't see a need for supported
property.

Thanks.
--Prasad