RE: [PATCH for-4.14 v3] mm: fix public declaration of struct xen_mem_acquire_resource

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Roger Pau Monne <roger.pau@xxxxxxxxxx>
> Sent: 26 June 2020 16:40
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: paul@xxxxxxx; Roger Pau Monne <roger.pau@xxxxxxxxxx>; Andrew Cooper 
> <andrew.cooper3@xxxxxxxxxx>;
> George Dunlap <george.dunlap@xxxxxxxxxx>; Ian Jackson 
> <ian.jackson@xxxxxxxxxxxxx>; Jan Beulich
> <jbeulich@xxxxxxxx>; Julien Grall <julien@xxxxxxx>; Stefano Stabellini 
> <sstabellini@xxxxxxxxxx>; Wei
> Liu <wl@xxxxxxx>
> Subject: [PATCH for-4.14 v3] mm: fix public declaration of struct 
> xen_mem_acquire_resource
> 
> XENMEM_acquire_resource and it's related structure is currently inside
> a __XEN__ or __XEN_TOOLS__ guarded section to limit it's scope to the
> hypervisor or the toolstack only. This is wrong as the hypercall is
> already being used by the Linux kernel at least, and as such needs to
> be public.
> 
> Also switch the usage of uint64_aligned_t to plain uint64_t, as
> uint64_aligned_t is only to be used by the toolstack. Doing such
> change will reduce the size of the structure on 32bit x86 by 4bytes,
> since there will be no padding added after the frame_list handle.
> 
> This is fine, as users of the previous layout will allocate 4bytes of
> padding that won't be read by Xen, and users of the new layout won't
> allocate those, which is also fine since Xen won't try to access them.
> 
> Note that the structure already has compat handling, and such handling
> will take care of copying the right size (ie: minus the padding) when
> called from a 32bit x86 context. This is true for the compat code both
> before and after this patch, since the structures in the memory.h
> compat header are subject to a pragma pack(4), which already removed
> the trailing padding that would otherwise be introduced by the
> alignment of the frame field to 8 bytes.
> 
> Fixes: 3f8f12281dd20 ('x86/mm: add HYPERVISOR_memory_op to acquire guest 
> resources')
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Release-acked-by: Paul Durrant <paul@xxxxxxx>

> ---
> Should also be backported.
> ---
> Changes since v2:
>  - Remove the tail padding.
>  - Expand commit message.
> 
> Changes since v1:
>  - Add padding on 32bits so structure size matches between arches and
>    the previous layout is kept.
> ---
>  xen/include/public/memory.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> index 850bd72c52..21057ed78e 100644
> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -610,6 +610,8 @@ struct xen_reserved_device_memory_map {
>  typedef struct xen_reserved_device_memory_map 
> xen_reserved_device_memory_map_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_reserved_device_memory_map_t);
> 
> +#endif /* defined(__XEN__) || defined(__XEN_TOOLS__) */
> +
>  /*
>   * Get the pages for a particular guest resource, so that they can be
>   * mapped directly by a tools domain.
> @@ -648,7 +650,7 @@ struct xen_mem_acquire_resource {
>       * IN - the index of the initial frame to be mapped. This parameter
>       *      is ignored if nr_frames is 0.
>       */
> -    uint64_aligned_t frame;
> +    uint64_t frame;
> 
>  #define XENMEM_resource_ioreq_server_frame_bufioreq 0
>  #define XENMEM_resource_ioreq_server_frame_ioreq(n) (1 + (n))
> @@ -669,8 +671,6 @@ struct xen_mem_acquire_resource {
>  typedef struct xen_mem_acquire_resource xen_mem_acquire_resource_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_mem_acquire_resource_t);
> 
> -#endif /* defined(__XEN__) || defined(__XEN_TOOLS__) */
> -
>  /*
>   * XENMEM_get_vnumainfo used by guest to get
>   * vNUMA topology from hypervisor.
> --
> 2.26.2