[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH 1/2] altp2m: Merge p2m_set_altp2m_mem_access and p2m_set_mem_access
- To: Jan Beulich <JBeulich@xxxxxxxx>
- From: "Lengyel, Tamas" <tlengyel@xxxxxxxxxxx>
- Date: Thu, 28 Jan 2016 07:34:41 -0700
- Cc: Wei Liu <wei.liu2@xxxxxxxxxx>, Ian Campbell <ian.campbell@xxxxxxxxxx>, Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxxxxx>, Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, Keir Fraser <keir@xxxxxxx>
- Delivery-date: Thu, 28 Jan 2016 14:34:52 +0000
- List-id: Xen developer discussion <xen-devel.lists.xen.org>
On Jan 28, 2016 6:18 AM, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
>
> >>> On 27.01.16 at 21:06, <tlengyel@xxxxxxxxxxx> wrote:
> > --- a/xen/include/public/hvm/hvm_op.h
> > +++ b/xen/include/public/hvm/hvm_op.h
> > @@ -431,18 +431,6 @@ 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);
> >
> > -struct xen_hvm_altp2m_set_mem_access {
> > -Â Â /* view */
> > -Â Â uint16_t view;
> > -Â Â /* Memory type */
> > -Â Â uint16_t hvmmem_access; /* xenmem_access_t */
> > -Â Â uint32_t pad;
> > -Â Â /* gfn */
> > -Â Â uint64_t gfn;
> > -};
> > -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);
>
> This is a guest visible interface, and hence can't be removed (nor
> be replaced by a tools only one).
What is your suggestion?
>
> > --- a/xen/include/public/memory.h
> > +++ b/xen/include/public/memory.h
> > @@ -423,11 +423,14 @@ struct xen_mem_access_op {
> >Â Â Â /* xenmem_access_t */
> >Â Â Â uint8_t access;
> >Â Â Â domid_t domid;
> > +Â Â uint16_t altp2m_idx;
>
> So this is a tools only interface, yes. But it's not versioned (other
> than e.g. domctl and sysctl), so altering the interface structure is
> at least fragile.
>
> And then, with this ...
>
> > --- a/xen/include/xen/p2m-common.h
> > +++ b/xen/include/xen/p2m-common.h
> > @@ -49,7 +49,8 @@ int unmap_mmio_regions(struct domain *d,
> >Â Â* If gfn == INVALID_GFN, sets the default access type.
> >Â Â*/
> >Â long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
> > -Â Â Â Â Â Â Â Â Â Â Â Â uint32_t start, uint32_t mask, xenmem_access_t access);
> > +Â Â Â Â Â Â Â Â Â Â Â Â uint32_t start, uint32_t mask, xenmem_access_t access,
> > +Â Â Â Â Â Â Â Â Â Â Â Â unsigned long altp2m_idx);
>
> ... why "unsigned long" instead of e.g. "unsigned int" here?
>
These were used interchangebly in the code, I've just picked on. The max value it can legitimetly have is 10 so there is not much difference in going with int instead of long.
Tamas
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
