Re: [Xen-devel] [PATCH v8 12/15] xen/arm: move arch specific grant table bits into grant_table.c

[Julien Grall <julien.grall@xxxxxxx>]

Hi Juergen,

On 20/09/17 07:34, Juergen Gross wrote:
> diff --git a/xen/include/asm-arm/grant_table.h 
> b/xen/include/asm-arm/grant_table.h
> index 0a248a765a..0870b5b782 100644
> --- a/xen/include/asm-arm/grant_table.h
> +++ b/xen/include/asm-arm/grant_table.h
> @@ -6,6 +6,10 @@
>   
>   #define INITIAL_NR_GRANT_FRAMES 4
>   
> +struct grant_table_arch {
> +    gfn_t *gfn;
> +};
> +
>   void gnttab_clear_flag(unsigned long nr, uint16_t *addr);
>   int create_grant_host_mapping(unsigned long gpaddr,
>           unsigned long mfn, unsigned int flags, unsigned int
> @@ -22,11 +26,19 @@ static inline int replace_grant_supported(void)
>       return 1;
>   }
>   
> -static inline void gnttab_set_frame_gfn(struct domain *d, unsigned long idx,
> -                                        gfn_t gfn)
> -{
> -    d->arch.grant_table_gfn[idx] = gfn;
> -} > +#define gnttab_init_arch(gt) 
    \
> +    ( ((gt)->arch.gfn = xzalloc_array(gfn_t, max_grant_frames)) == 0     \
I am not sure to understand the 0 here. Don't you check the return of 
xzalloc_array? If so it should be NULL if it failed. And therefore the 
return error looks inverted below.
> +      ? 0 : -ENOMEM )
I admit I would much prefer to see static inline rather than define. 
More typesafe and usually easier to read. You cannot do that because the 
type of gt is only defined in grant-table?
Nonetheless, I think you could clarify this code by doing:

({
   (gt)->arch.gfn = xzalloc_array(....);
   ( g->arch.gfn ? 0 : -ENOMEM );
)}

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel