Re: [PATCH 2/2] iommu/amd-vi: do not zero IOMMU MMIO region

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.05.2026 10:43, Roger Pau Monné wrote:
> On Wed, May 06, 2026 at 10:28:52AM +0200, Jan Beulich wrote:
>> On 06.05.2026 09:37, Roger Pau Monne wrote:
>>> @@ -1381,6 +1372,11 @@ static int __init amd_iommu_prepare_one(struct 
>>> amd_iommu *iommu)
>>>      if ( amd_iommu_max_paging_mode < amd_iommu_min_paging_mode )
>>>          return -ERANGE;
>>>  
>>> +    /* Read current control register and forcefully disable the IOMMU. */
>>> +    iommu->ctrl.raw = readq(iommu->mmio_base + IOMMU_CONTROL_MMIO_OFFSET);
>>> +    disable_iommu(iommu, true);
>>
>> Don't you also need to pre-fill iommu->features?
> 
> Indeed, that's done just ahead of this chunk, in the
> get_iommu_features() call.
> 
>> And with that field's use in
>> disable_iommu(), won't we be at risk of leaving stuff enabled which we are
>> entirely unaware of?
> 
> Possibly, yes, that's always a risk.
> 
>> Even if we fully cleared the control register (which
>> would eliminate the need to fetch features), down the road a 2nd control
>> register could appear.
> 
> We do clear the control register, it's indirectly done by us setting
> iommu->ctrl.raw = 0 after the disable_iommu() call.
> 
> I did wonder about just doing a write of 0 to the control register,
> but I think it's best if we try to gracefully disable the features (as
> done in disable_iommu()), and then reset the cached control state to
> 0.  Future writes to the control register will clear any bits not
> directly set by Xen.

Maybe better to explicitly write out that 0 right away, even if you want
to keep using disable_iommu()?

Jan