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

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

> On 12 Feb 2025, at 12:37, Luca Fancellu <Luca.Fancellu@xxxxxxx> wrote:
> 
> Hi Andrew,
> 
> thanks for your review,
> 
>> On 12 Feb 2025, at 11:50, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>> 
>> On 12/02/2025 9:18 am, Luca Fancellu wrote:
>>> When Xen is built without HAS_PASSTHROUGH, there are some parts
>>> in arm and x86 where iommu_* functions are called in the codebase,
>>> but their implementation is under xen/drivers/passthrough that is
>>> not built.
>>> 
>>> So provide some stub for these functions in order to build Xen
>>> when !HAS_PASSTHROUGH, which is the case for example on systems
>>> with MPU support.
>>> 
>>> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
>>> ---
>>> xen/arch/arm/include/asm/grant_table.h |  8 ++++++
>>> xen/include/xen/iommu.h                | 40 +++++++++++++++++++++++---
>>> 2 files changed, 44 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/xen/arch/arm/include/asm/grant_table.h 
>>> b/xen/arch/arm/include/asm/grant_table.h
>>> index d3c518a926b9..e21634b752df 100644
>>> --- a/xen/arch/arm/include/asm/grant_table.h
>>> +++ b/xen/arch/arm/include/asm/grant_table.h
>>> @@ -73,9 +73,17 @@ int replace_grant_host_mapping(uint64_t gpaddr, mfn_t 
>>> frame,
>>> #define gnttab_status_gfn(d, t, i)                                       \
>>>    page_get_xenheap_gfn(gnttab_status_page(t, i))
>>> 
>>> +#ifdef CONFIG_HAS_PASSTHROUGH
>>> +
>>> #define gnttab_need_iommu_mapping(d)                    \
>>>    (is_domain_direct_mapped(d) && is_iommu_enabled(d))
>>> 
>>> +#else
>>> +
>>> +#define gnttab_need_iommu_mapping(d) (false)
>> 
>> This doesn't evaluate d, which can lead to other build problems.
>> 
>> Instead of providing two, you should insert
>> "IS_ENABLED(CONFIG_HAS_PASSTHROUGH) &&" into the existing
>> gnttab_need_iommu_mapping().
> 
> I’ll do that for this case, I already checked and it works well, just for my 
> knowledge could you
> explain to me what build problems can happen? Is it something related to the 
> compiler that
> doesn’t see a usage for d?
> 
> 
>> 
>> The same applies to several other hunks too.
> 
> Are you referring to iommu_use_hap_pt? I have to say that I’ve tried before 
> to insert another
> IS_ENABLED(…) but it was failing the compilation because without 
> HAS_PASSTHROUGH
> dom_iommu(d) is (&(d)->iommu), but the iommu field doesn’t exists.
> 
> So I’m not sure how to proceed there, do you have any suggestions?

Oh sorry, nevermind this point, I see I can maybe use the same approach as 
need_iommu_pt_sync(d)

> 
> Cheers,
> Luca