Re: [PATCH v4 3/4] xen: implement guest_physmap_(un)pin_range

[Ian Campbell <Ian.Campbell@citrix.com>]

On Fri, 2013-09-06 at 17:06 +0100, Stefano Stabellini wrote:
> On Mon, 19 Aug 2013, Ian Campbell wrote:
> > > +{
> > > +    lpae_t pte = *ptep;
> > > +    if ( pte.p2m.avail & P2M_DMA_PIN )
> > > +        return -EINVAL;
> > > +    pte.p2m.avail |= P2M_DMA_PIN;
> > 
> > I think test_and_set_bit would be better here and you should operate on
> > ptep directly IMHO rather than changing a copy and writing that. Might
> > require a pte_modified macro with the flushing stuff from write_pte.
> > 
> 
> Considering that ptep->p2m.avail is a bit-field and that
> test_and_set_bit operates on a unsigned long pointer (one cannot get a
> pointer to a bit-field), and considering that avail is at bit 55 > 32
> (we would need to take care of incrementing the pointer too),

No you don't, test_and_set_bit can cope with bitfields bigger than
wordsize.

All you would need is 
	#define P2M_DMA_PIN 54
	test_and_set_bit(P2M_DMA_PIN, &ptr.bits)

>  I think
> that it's not worth making this change because it would make the code
> far less readable.
> 
> In any case these two functions are called with the p2m lock held, so
> the current implementation should be safe. 

Ah, in that case you can do either as you wish.

Ian