[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 2/4] xen: introduce grant_map_exists

>>> On 02.10.14 at 13:41, <tim@xxxxxxx> wrote:
> At 11:42 +0100 on 02 Oct (1412246555), Stefano Stabellini wrote:
>> On Thu, 2 Oct 2014, Tim Deegan wrote:
>> > Hi,
>> > 
>> > At 11:02 +0100 on 02 Oct (1412244156), Stefano Stabellini wrote:
>> > > Check whether an mfn has been granted to a given domain on a target
>> > > grant table.
>> > 
>> > The implementation looks good but I'm concerned about the need for it,
>> > since linear scans of busy grant tables could get fairly expensive.
>> > 
>> > I see in patch 3/4 you add a hypercall to flush caches, taking an mfn
>> > range.  Should that not take a gfn range?  That would be the idiomatic
>> > interface, and the p2m lookup would tell you whether the guest had
>> > appropiate rights.
>> > 
>> > I suspect, from reading 0/4, that I'm missing something about the
>> > tangle of non-IOMMU dom0 operations on ARM. :)
>> The hypercall is going to be used to flush foreign pages grant mapped in
>> Dom0. Unfortunately dom0 doesn't know the gfn, only the dma address
>> (==mfn).
> Blargh.  Silly linux, forgetting its grant handles. :)
> So after some IRL discussion with Stefano and IanC, I'm convinced that
> the alternatives (linux maintaining per-map lookup tables to get back
> from DMA address to grant handle) aren't going to work.  But we can
> make this API a bit neater.  I think the design we came up with is:
>  - the cache-flush hypercall becomes a grant-table op instead of a
>    memop.
>  - the argument becomes a 'dev_bus_addr' instead of an MFN.  That is,
>    it must be the address returned in the dev_bus_addr field of a
>    grant map operation.  If not, it will return EINVAL.

Then at the very least let's allow for either a dev_bus_addr or
a grant-ref (even if for the moment the latter may yield
-EOPNOTSUPP in order to not spend more time on this than
immediately necessary). That way guests remembering what
they did a few microseconds back can do this in a more well
behaved fashion.


>  - the interals are shuffled so that the grant code calls out to
>    arch-specific code to do the cache flush, which solves the locking
>    issue.  Unlocking before the flush is probably the right thing to
>    do anyway, but we can avoid exposing this new grant_map_exists()
>    function that might encourage people to write racy code.
> Cheers,
> Tim.

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.