Re: [Xen-devel] [PATCH v4] x86/altp2m: Add a subop for obtaining the mem access of a page

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Tue, Sep 25, 2018 at 9:04 AM Razvan Cojocaru
<rcojocaru@xxxxxxxxxxxxxxx> wrote:
>
> On 9/19/18 6:29 PM, Tamas K Lengyel wrote:
> > On Mon, Sep 3, 2018 at 9:47 AM Adrian Pop <apop@xxxxxxxxxxxxxxx> wrote:
> >> diff --git a/xen/include/public/hvm/hvm_op.h 
> >> b/xen/include/public/hvm/hvm_op.h
> >> index bbba99e5f5..bbb0aa984a 100644
> >> --- a/xen/include/public/hvm/hvm_op.h
> >> +++ b/xen/include/public/hvm/hvm_op.h
> >> @@ -234,6 +234,7 @@ struct xen_hvm_altp2m_view {
> >>  typedef struct xen_hvm_altp2m_view xen_hvm_altp2m_view_t;
> >>  DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_view_t);
> >>
> >> +#if __XEN_INTERFACE_VERSION__ < 0x00040a00
> >>  struct xen_hvm_altp2m_set_mem_access {
> >>      /* view */
> >>      uint16_t view;
> >> @@ -245,6 +246,19 @@ struct xen_hvm_altp2m_set_mem_access {
> >>  };
> >>  typedef struct xen_hvm_altp2m_set_mem_access 
> >> xen_hvm_altp2m_set_mem_access_t;
> >>  DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_set_mem_access_t);
> >> +#endif /* __XEN_INTERFACE_VERSION__ < 0x00040a00 */
> >> +
> >> +struct xen_hvm_altp2m_mem_access {
> >> +    /* view */
> >> +    uint16_t view;
> >> +    /* Memory type */
> >> +    uint16_t hvmmem_access; /* xenmem_access_t */
> >
> > A structure name with "mem_access" having a variable name
> > "hvmmem_access" and a comment saying "xenmem_access". This is
> > confusing. I understand that you copy/pasted this from the existing op
> > but it doesn't look good. Perhaps fix both if we are touching it?
> > Also, in public/memory.h the width of access is uint8_t so I'm not
> > sure why the discrepancy.
>
> The uint16_t has been probably chosen to be that because it simplifies
> padding. I can rename "hvmmem_access" to simply "access". Should I just
> remove the comment, or do you think something else is more appropriate?

Just renaming the variable should suffice, the comment is helpful to
explain what values go in the field.

Thanks,
Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel