Re: [PATCH v2 2/2] xen/arm: gnttab: modify macros to evaluate all arguments and only once

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Julien,

On 16.05.2022 12:19, Julien Grall wrote:
> Hi Michal,
> 
> On 06/05/2022 10:42, Michal Orzel wrote:
>> Modify macros to evaluate all the arguments and make sure the arguments
>> are evaluated only once. Introduce following intermediate macros:
>> gnttab_status_gfn_, gnttab_shared_gfn_ that do not take domain as a
>> parameter. These are to be used locally and allow us to avoid passing
>> NULL from gnttab_get_frame_gfn to the respective macros (without _ suffix).
>> Make use of a domain parameter from gnttab_shared_gfn and gnttab_status_gfn
>> by adding an ASSERT.
>>
>> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
> 
> Most of the helpers below are going to disappear with Oleksandr latest work 
> (see [1]).
> 
> Looking at Oleksandr's patch, I think only gnttab_set_frame_gfn() would end 
> up to use one of the macro parameters twice. So I would like to suggest to 
> chat with Oleksandr if we can tweak his patch (can be done on commit) or we 
> rebase this patch on top of his work.
> 
> Cheers,
> 
> [1] 
> https://lore.kernel.org/xen-devel/1652294845-13980-1-git-send-email-olekstysh@xxxxxxxxx/
> 

By looking at Oleksandr patch:
1. there are 2 macros: gnttab_set_frame_gfn, gnttab_need_iommu_mapping that use 
one of the macro parameters twice.
2. gnttab_get_frame_gfn still passes NULL as a domain parameter to 
gnttab_shared_gfn/gnttab_status_gfn that do not evaluate domain parameter

I agree that point 1 could be fixed on commit but point 2 requires in my 
opinion adding intermediate macros to avoid passing NULL (just like I did).

As this would require more work from Oleksandr, I'm ok to rebase my patch on 
top of his work once merged.

Cheers,
Michal