Re: [Xen-devel] [PATCH 07 of 10] Transitive grant support

[Steven Smith <steven.smith@xxxxxxxxxx>]

> >>> -    unsigned long frame;  /* Frame being granted.                     */
> >>> +    domid_t       trans_dom;
> >>> +    uint32_t      trans_gref;
> >>> +    uint32_t      frame;  /* Frame being granted.                     */
> >> 
> >> Using a uint32_t for a frame number is not valid anymore after the recent
> >> sparse physical memory support changes (and I don't think it would have
> >> been valid on ia64 even before).
Yes, agreed, I'll put it back to unsigned long.

> >What about the 'uint32_t frame' in grant_entry_v2_t -- I'm surprised you
> >didn't mention that also? It's most critical we get the sizing of that
> 
> Oops, that one slipped my attention.
> 
> >correct, while we're rev'ing a public interface. Given the definition of
> >__spacer[], it looks like there's space to have a full 'uint64_t frame' in
> >that structure too, no problem. A bit of field reordering might be needed to
> >make everything align naturally yet compactly, is all, I think?
> Agreed. Not sure though whether uintXX_t is the right thing to use -
> after all we have xen_pfn_t for exactly this purpose.
Maybe.  xen_pfn_t probably would be the right type, but, again, it's
not 32/64 clean, which means it'd need some compat stuff.  Not
impossible, but kind of annoying.

Would something like this be acceptable?

struct grant_entry_v2 {
    grant_entry_header_t hdr;
    union {
        /*
         * The frame to which we are granting access.  This field has
         * the same meaning as the grant_entry_v1 field of the same
         * name.
         */
        struct {
            uint32_t pad0;
            uint64_t frame;
        } __attribute__((packed));

        /*
         * If the grant type is GTF_grant_access and GTF_sub_page is
         * set, @domid is allowed to access bytes
         * [@page_off,@page_off+@length) in frame @frame.
         */
        struct {
            uint16_t page_off;
            uint16_t length;
            uint64_t frame;
        } __attribute__((packed)) sub_page;

        /*
         * If the grant is GTF_transitive, @domid is allowed to use
         * the grant @gref in domain @trans_domid, as if it was the
         * local domain.  Obviously, the transitive access must be
         * compatible with the original grant.
         *
         * The current version of Xen does not allow transitive grants
         * to be mapped.
         */
        struct {
            domid_t trans_domid;
            uint16_t pad0;
            grant_ref_t gref;
        } transitive;

        uint32_t __spacer[3]; /* Pad to a power of two */
    };
};
typedef struct grant_entry_v2 grant_entry_v2_t;



The __attribute__((packed))s are very much not a thing of beauty, but
it's at least fairly self-contained.

Steven.

Attachment: signature.asc
Description: Digital signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel