Re: [Xen-devel] [PATCH] add HVMOP_get_mem_type hvmop

[Tim Deegan <Tim.Deegan@xxxxxxxxxx>]

At 14:59 +0100 on 03 May (1304434768), Olaf Hering wrote:
> On Tue, May 03, Tim Deegan wrote:
> 
> > > +        rc = rcu_lock_remote_target_domain_by_id(a.domid, &d);
> > 
> > I thought this call was intended to be used from inside the guest in
> > question.  rcu_lock_remote_target_domain_by_id() explicitly refuses to
> > let a domain operate on itself. 
> 
> Hmm, I have to test it with xen-unstable too. I did copy&paste from
> other parts of the file. 4.0 used rcu_lock_target_domain_by_id, so can I
> use that function?

Yes; I think that's the right one for you.  It's the same except for the
check that the target isn't the caller. 

> > > +typedef enum {
> > > +    HVMMEM_ram_rw,             /* Normal read/write guest RAM */
> > > +    HVMMEM_ram_ro,             /* Read-only; writes are discarded */
> > > +    HVMMEM_mmio_dm,            /* Reads and write go to the device model 
> > > */
> > > +} hvmmem_type_t;
> > > +
> > 
> > This is now outside the #ifdef, when both of its users are inside it.
> > If that wasn't deliberate, please put it back. 
> 
> Should HVMOP_get_mem_type use hvmmem_type_t? Its outside the ifdef,

Looks to me like it's inside; at least, inside an equivalent one.  Did
you compile-test your kernel against this patch?

> otherwise unmodified_drivers/linux-2.6/platform-pci/platform-pci.c does
> not get the define.

OK; as long as both the typedef and its user are outside the ifdef,
that's fine.

> > > +#define HVMOP_get_mem_type    15
> > > +/* Return hvmmem_type_t for the specified pfn. */
> > > +struct xen_hvm_get_mem_type {
> > > +    /* Domain to be queried. */
> > > +    domid_t domid;
> > > +    /* OUT variable. */
> > > +    uint8_t mem_type;
> > > +    /* IN variable. */
> > > +    uint64_t pfn;
> > 
> > This structure will be laid out differently on 32-bit and 64-bit
> > builds. :(  Also, since the _set operation uses a 16-bit variable for
> > the type, you might as well do the same here. 
> 
> Good point. Should I use the same padding as in xen_hvm_pagetable_dying?
> 
> struct xen_hvm_get_mem_type {
>     /* Domain to be queried. */
>     domid_t domid;
>     /* OUT variable. */
>     uint16_t mem_type;
>     uint16_t pad[2];
>     /* IN variable. */
>     uint64_t pfn;
> };

Yes, that's fine.

Tim.

-- 
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

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