Re: [PATCH v3 1/2] xen/passthrough: Provide stub functions when !HAS_PASSTHROUGH

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.02.2025 16:25, Luca Fancellu wrote:
>> On 19 Feb 2025, at 13:30, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>> On 19.02.2025 14:06, Luca Fancellu wrote:
>>>> On 19 Feb 2025, at 12:45, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>> As per the
>>>> respective revlog entry, this change looks to belong into whatever is
>>>> going to be done to deal with the one Arm use of the macro. Or maybe
>>>> it's unneeded altogether.
>>>
>>> I didn’t understand that you were opposing to protecting iommu_use_hap_pt() 
>>> when
>>> !HAS_PASSTHROUGH, I thought you were referring only to the stub in the #else
>>> branch.
>>> Can I ask why?
>>
>> Sure. And no, I'm not against the extra protection. I'm against unnecessary
>> code churn. That is, any such a re-arrangement wants to have some kind of
>> justification.
> 
> ok, yes the justification is that MPU system will be built with 
> !HAS_PASSTHROUGH,
> but there is a common function (p2m_set_way_flush) to MMU/MPU subsystem that
> I would like to keep common, to do so I have to hide the macro in this 
> particular
> configuration and afterwards I have two choices:
> 
> 1) provide a stub implementation on the Arm side
> 2) provide a stub implementation in iommu.h
> 
> number 2 felt better because it could be applied on any Xen configuration 
> without
> HAS_PASSTHROUGH, even if at the moment there is only MPU.
> 
> Number 1 let the possibility for the specific configuration to choose what to 
> do in absence
> of HAS_PASSTHROUGH.
> 
> Now I would like your view on what would be acceptable here.

I think I indicated earlier that I'd like the Arm maintainers to voice
their preference. Doing it in iommu.h may be okay, but also may not be.
Yet to decide that very Arm use of the macro needs taking into account,
and I lack context there.

>>> in any case when !HAS_PASSTHROUGH, this macro is not usable
>>> since dom_iommu() is resolved to a membed that doesn’t exist in the 
>>> configuration,
>>> am I missing something?
>>
>> You very likely aren't, yet the macro's presence also does no harm. We
>> have lots of macros and declarations which are usable only in certain
>> configurations. Sometimes this just happens to be that way, sometimes it's
>> actually deliberate (e.g. to facilitate DCE).
> 
> Ok, in this particular case, as I explained above, this macro is one of the 
> thing preventing
> Arm MPU side to build, otherwise I wouldn’t have touched it.

Yes, except that this wasn't said anywhere. Also if you mean to take
care of this macro here, then in full please. I.e. either don't touch
that area of the header at all, or provide (wherever suitable) a
stub alongside moving the #ifdef.

Jan