All of lore.kernel.org
 help / color / mirror / Atom feed
* XENMAPSPACE_grant_table vs. GNTTABOP_setup_table
@ 2020-06-09  9:44 Martin Lucina
  2020-06-09 10:22 ` Andrew Cooper
  0 siblings, 1 reply; 9+ messages in thread
From: Martin Lucina @ 2020-06-09  9:44 UTC (permalink / raw)
  To: xen-devel, mirageos-devel

Hi,

I've been making progress on bootstrapping a new, PVHv2 only, Xen platform
stack for MirageOS [1]. The basics are now functional and I have started to
re-implement the grant table code.

After studying both the Mini-OS and Linux implementations some, I don't
understand the difference between using XENMAPSPACE_grant_table vs.
GNTTABOP_setup_table to set up the initial grant table mapping for the
guest.

Are these calls just newer and older ways of accomplishing the same thing?
The Linux driver seems to use both in various conditions, whereas Mini-OS
uses the former on ARM and the latter on x86.

If these are functionally equivalent, then for my purposes I'd rather use
XENMAPSPACE_setup_table, since IIUC this lets me map the grant table at an
address of my choosing rather than GNTTABOP_setup_table which at least on
x86_64 appears to be returning PFNs at the top of the address space.

Any advice much appreciated,

-mato

[1] https://github.com/mirage/mirage/issues/1159


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: XENMAPSPACE_grant_table vs. GNTTABOP_setup_table
  2020-06-09  9:44 XENMAPSPACE_grant_table vs. GNTTABOP_setup_table Martin Lucina
@ 2020-06-09 10:22 ` Andrew Cooper
  2020-06-09 10:50   ` Anil Madhavapeddy
  2020-06-10 13:22   ` Martin Lucina
  0 siblings, 2 replies; 9+ messages in thread
From: Andrew Cooper @ 2020-06-09 10:22 UTC (permalink / raw)
  To: Martin Lucina, xen-devel, mirageos-devel

On 09/06/2020 10:44, Martin Lucina wrote:
> Hi,
>
> I've been making progress on bootstrapping a new, PVHv2 only, Xen platform
> stack for MirageOS [1]. The basics are now functional and I have started to
> re-implement the grant table code.
>
> After studying both the Mini-OS and Linux implementations some, I don't
> understand the difference between using XENMAPSPACE_grant_table vs.
> GNTTABOP_setup_table to set up the initial grant table mapping for the
> guest.
>
> Are these calls just newer and older ways of accomplishing the same thing?
> The Linux driver seems to use both in various conditions, whereas Mini-OS
> uses the former on ARM and the latter on x86.
>
> If these are functionally equivalent, then for my purposes I'd rather use
> XENMAPSPACE_setup_table, since IIUC this lets me map the grant table at an
> address of my choosing rather than GNTTABOP_setup_table which at least on
> x86_64 appears to be returning PFNs at the top of the address space.
>
> Any advice much appreciated,

There is a little bit of history here...

GNTTABOP_setup_table was the original PV way of doing things (specify
size as an input, get a list of frames as an output to map), and
XENMAPSPACE_grant_table was the original HVM way of doing things (as
mapping is the other way around - I specify a GFN which I'd like to turn
into a grant table mapping).

When grant v2 came along, it was only XENMAPSPACE_grant_table updated to
be compatible.  i.e. you have to use XENMAPSPACE_grant_table to map the
status frames even if you used GNTTABOP_setup_table previously.

It is a mistake that GNTTABOP_setup_table was usable in HVM guests to
being with.  Returning -1 is necessary to avoid an information leak (the
physical address of the frames making up the grant table) which an HVM
guest shouldn't, and has no use knowing.

An with that note, ARM is extra special because the grant API is
specified to use host physical addresses rather than guest physical (at
least for dom0, for reasons of there generally not being an IOMMU),
which is why it does use the old PV way.

It is all a bit of a mess.

~Andrew


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: XENMAPSPACE_grant_table vs. GNTTABOP_setup_table
  2020-06-09 10:22 ` Andrew Cooper
@ 2020-06-09 10:50   ` Anil Madhavapeddy
  2020-06-09 11:29     ` Julien Grall
  2020-06-10 13:22   ` Martin Lucina
  1 sibling, 1 reply; 9+ messages in thread
From: Anil Madhavapeddy @ 2020-06-09 10:50 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: xen-devel, Martin Lucina, mirageos-devel

[-- Attachment #1: Type: text/plain, Size: 2764 bytes --]

On 9 Jun 2020, at 11:22, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> 
> On 09/06/2020 10:44, Martin Lucina wrote:
>> Hi,
>> 
>> I've been making progress on bootstrapping a new, PVHv2 only, Xen platform
>> stack for MirageOS [1]. The basics are now functional and I have started to
>> re-implement the grant table code.
>> 
>> After studying both the Mini-OS and Linux implementations some, I don't
>> understand the difference between using XENMAPSPACE_grant_table vs.
>> GNTTABOP_setup_table to set up the initial grant table mapping for the
>> guest.
>> 
>> Are these calls just newer and older ways of accomplishing the same thing?
>> The Linux driver seems to use both in various conditions, whereas Mini-OS
>> uses the former on ARM and the latter on x86.
>> 
>> If these are functionally equivalent, then for my purposes I'd rather use
>> XENMAPSPACE_setup_table, since IIUC this lets me map the grant table at an
>> address of my choosing rather than GNTTABOP_setup_table which at least on
>> x86_64 appears to be returning PFNs at the top of the address space.
>> 
>> Any advice much appreciated,
> 
> There is a little bit of history here...
> 
> GNTTABOP_setup_table was the original PV way of doing things (specify
> size as an input, get a list of frames as an output to map), and
> XENMAPSPACE_grant_table was the original HVM way of doing things (as
> mapping is the other way around - I specify a GFN which I'd like to turn
> into a grant table mapping).
> 
> When grant v2 came along, it was only XENMAPSPACE_grant_table updated to
> be compatible.  i.e. you have to use XENMAPSPACE_grant_table to map the
> status frames even if you used GNTTABOP_setup_table previously.
> 
> It is a mistake that GNTTABOP_setup_table was usable in HVM guests to
> being with.  Returning -1 is necessary to avoid an information leak (the
> physical address of the frames making up the grant table) which an HVM
> guest shouldn't, and has no use knowing.
> 
> An with that note, ARM is extra special because the grant API is
> specified to use host physical addresses rather than guest physical (at
> least for dom0, for reasons of there generally not being an IOMMU),
> which is why it does use the old PV way.

Thanks, that makes sense. It doesn't seem too much of a mess from the guest
perspective to use just XENMAPSPACE_grant_table exclusively then.  With
Martin's work, the MirageOS Xen backend should just use the latest and greatest
APIs, with no backwards compatibility to older Xen versions required.

I'm still a little confused about ARM though -- is there an equivalent of
XENMAPSPACE_grant_table there? It sounds like we can't leave the
GNTTABOP macros behind entirely...

regards,
Anil

[-- Attachment #2: Type: text/html, Size: 18667 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: XENMAPSPACE_grant_table vs. GNTTABOP_setup_table
  2020-06-09 10:50   ` Anil Madhavapeddy
@ 2020-06-09 11:29     ` Julien Grall
  0 siblings, 0 replies; 9+ messages in thread
From: Julien Grall @ 2020-06-09 11:29 UTC (permalink / raw)
  To: Anil Madhavapeddy, Andrew Cooper; +Cc: xen-devel, Martin Lucina, mirageos-devel



On 09/06/2020 11:50, Anil Madhavapeddy wrote:
> On 9 Jun 2020, at 11:22, Andrew Cooper <andrew.cooper3@citrix.com 
> <mailto:andrew.cooper3@citrix.com>> wrote:
>>
>> On 09/06/2020 10:44, Martin Lucina wrote:
>>> Hi,
>>>
>>> I've been making progress on bootstrapping a new, PVHv2 only, Xen 
>>> platform
>>> stack for MirageOS [1]. The basics are now functional and I have 
>>> started to
>>> re-implement the grant table code.
>>>
>>> After studying both the Mini-OS and Linux implementations some, I don't
>>> understand the difference between using XENMAPSPACE_grant_table vs.
>>> GNTTABOP_setup_table to set up the initial grant table mapping for the
>>> guest.
>>>
>>> Are these calls just newer and older ways of accomplishing the same 
>>> thing?
>>> The Linux driver seems to use both in various conditions, whereas Mini-OS
>>> uses the former on ARM and the latter on x86.
>>>
>>> If these are functionally equivalent, then for my purposes I'd rather use
>>> XENMAPSPACE_setup_table, since IIUC this lets me map the grant table 
>>> at an
>>> address of my choosing rather than GNTTABOP_setup_table which at least on
>>> x86_64 appears to be returning PFNs at the top of the address space.
>>>
>>> Any advice much appreciated,
>>
>> There is a little bit of history here...
>>
>> GNTTABOP_setup_table was the original PV way of doing things (specify
>> size as an input, get a list of frames as an output to map), and
>> XENMAPSPACE_grant_table was the original HVM way of doing things (as
>> mapping is the other way around - I specify a GFN which I'd like to turn
>> into a grant table mapping).
>>
>> When grant v2 came along, it was only XENMAPSPACE_grant_table updated to
>> be compatible.  i.e. you have to use XENMAPSPACE_grant_table to map the
>> status frames even if you used GNTTABOP_setup_table previously.
>>
>> It is a mistake that GNTTABOP_setup_table was usable in HVM guests to
>> being with.  Returning -1 is necessary to avoid an information leak (the
>> physical address of the frames making up the grant table) which an HVM
>> guest shouldn't, and has no use knowing.
>>
>> An with that note, ARM is extra special because the grant API is
>> specified to use host physical addresses rather than guest physical (at
>> least for dom0, for reasons of there generally not being an IOMMU),
>> which is why it does use the old PV way.
> 
> Thanks, that makes sense. It doesn't seem too much of a mess from the guest
> perspective to use just XENMAPSPACE_grant_table exclusively then.  With
> Martin's work, the MirageOS Xen backend should just use the latest and 
> greatest
> APIs, with no backwards compatibility to older Xen versions required.
> 
> I'm still a little confused about ARM though -- is there an equivalent of
> XENMAPSPACE_grant_table there? It sounds like we can't leave the
> GNTTABOP macros behind entirely...

XENMAPSPACE_grant_table exists and works perfectly fine on Arm. It is 
using guest physical address as it should.

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: XENMAPSPACE_grant_table vs. GNTTABOP_setup_table
  2020-06-09 10:22 ` Andrew Cooper
  2020-06-09 10:50   ` Anil Madhavapeddy
@ 2020-06-10 13:22   ` Martin Lucina
  2020-06-10 13:40     ` Andrew Cooper
  1 sibling, 1 reply; 9+ messages in thread
From: Martin Lucina @ 2020-06-10 13:22 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: xen-devel, mirageos-devel

On Tuesday, 09.06.2020 at 11:22, Andrew Cooper wrote:
> There is a little bit of history here...
> 
> GNTTABOP_setup_table was the original PV way of doing things (specify
> size as an input, get a list of frames as an output to map), and
> XENMAPSPACE_grant_table was the original HVM way of doing things (as
> mapping is the other way around - I specify a GFN which I'd like to turn
> into a grant table mapping).
> 
> When grant v2 came along, it was only XENMAPSPACE_grant_table updated to
> be compatible.  i.e. you have to use XENMAPSPACE_grant_table to map the
> status frames even if you used GNTTABOP_setup_table previously.
> 
> It is a mistake that GNTTABOP_setup_table was usable in HVM guests to
> being with.  Returning -1 is necessary to avoid an information leak (the
> physical address of the frames making up the grant table) which an HVM
> guest shouldn't, and has no use knowing.
> 
> An with that note, ARM is extra special because the grant API is
> specified to use host physical addresses rather than guest physical (at
> least for dom0, for reasons of there generally not being an IOMMU),
> which is why it does use the old PV way.
> 
> It is all a bit of a mess.

Thanks for explaining, this is helpful.

So, going with the grant v2 ABI, is there a modern equivalent of
GNTTABOP_get_status_frames? Reading memory.h I'm guessing that it might be
XENMEM_add_to_physmap with space=XENMAPSPACE_grant_table and
idx=(XENMAPIDX_grant_table_status + N) where N is the frame I want, but
this is not explicitly mentioned anywhere and Linux uses the GNTTABOP
mechanism.

Further to that, what is the format of the grant status frames?
grant_table.h doesn't have much to say about it.

And lastly, given that I want the v2 grant ABI exclusively, I presume it's
sufficient to call GNTTABOP_set_version (version=2) first thing and abort
if it failed? Presumably the default is always v1 at start of day?

Thanks,

-mato


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: XENMAPSPACE_grant_table vs. GNTTABOP_setup_table
  2020-06-10 13:22   ` Martin Lucina
@ 2020-06-10 13:40     ` Andrew Cooper
  2020-06-10 13:56       ` Martin Lucina
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Cooper @ 2020-06-10 13:40 UTC (permalink / raw)
  To: Martin Lucina, xen-devel, mirageos-devel

On 10/06/2020 14:22, Martin Lucina wrote:
> On Tuesday, 09.06.2020 at 11:22, Andrew Cooper wrote:
>> There is a little bit of history here...
>>
>> GNTTABOP_setup_table was the original PV way of doing things (specify
>> size as an input, get a list of frames as an output to map), and
>> XENMAPSPACE_grant_table was the original HVM way of doing things (as
>> mapping is the other way around - I specify a GFN which I'd like to turn
>> into a grant table mapping).
>>
>> When grant v2 came along, it was only XENMAPSPACE_grant_table updated to
>> be compatible.  i.e. you have to use XENMAPSPACE_grant_table to map the
>> status frames even if you used GNTTABOP_setup_table previously.
>>
>> It is a mistake that GNTTABOP_setup_table was usable in HVM guests to
>> being with.  Returning -1 is necessary to avoid an information leak (the
>> physical address of the frames making up the grant table) which an HVM
>> guest shouldn't, and has no use knowing.
>>
>> An with that note, ARM is extra special because the grant API is
>> specified to use host physical addresses rather than guest physical (at
>> least for dom0, for reasons of there generally not being an IOMMU),
>> which is why it does use the old PV way.
>>
>> It is all a bit of a mess.
> Thanks for explaining, this is helpful.
>
> So, going with the grant v2 ABI, is there a modern equivalent of
> GNTTABOP_get_status_frames? Reading memory.h I'm guessing that it might be
> XENMEM_add_to_physmap with space=XENMAPSPACE_grant_table and
> idx=(XENMAPIDX_grant_table_status + N) where N is the frame I want, but
> this is not explicitly mentioned anywhere and Linux uses the GNTTABOP
> mechanism.
>
> Further to that, what is the format of the grant status frames?
> grant_table.h doesn't have much to say about it.
>
> And lastly, given that I want the v2 grant ABI exclusively, I presume it's
> sufficient to call GNTTABOP_set_version (version=2) first thing and abort
> if it failed? Presumably the default is always v1 at start of day?

What kind of guests are you trying to target here?

Since my reply, I tried to experiment, and I think you're forced to use
GNTTABOP_setup_table/GNTTABOP_get_status_frames for x86 PV guests, and
XENMEM_add_to_physmap for x86 HVM/PVH guests.

You can't depend on version 2 being available.  Its not available for
ARM at all, and may be disabled for security reasons on x86 (there was
some extended fun with transitive grants in the past, and we offered
"totally disable grant v2" as one mitigation).

Use v1 unless you have a specific need to use v2 (transitive grants or
subpage copies, or support for >16TB VMs (HVM) or hosts (PV)).  Amongst
other things, its far more simple to use correctly.  (v2 devolves to
infinite loops to use correctly, because there is no way to do an atomic
option covering both the flags and status entries at the same time.)

If you need to use v2, you must cleanly cope with it not being
available, and fall back to v1.

~Andrew


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: XENMAPSPACE_grant_table vs. GNTTABOP_setup_table
  2020-06-10 13:40     ` Andrew Cooper
@ 2020-06-10 13:56       ` Martin Lucina
  2020-06-10 14:21         ` Andrew Cooper
  0 siblings, 1 reply; 9+ messages in thread
From: Martin Lucina @ 2020-06-10 13:56 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: xen-devel, mirageos-devel

On Wednesday, 10.06.2020 at 14:40, Andrew Cooper wrote:
> > So, going with the grant v2 ABI, is there a modern equivalent of
> > GNTTABOP_get_status_frames? Reading memory.h I'm guessing that it might be
> > XENMEM_add_to_physmap with space=XENMAPSPACE_grant_table and
> > idx=(XENMAPIDX_grant_table_status + N) where N is the frame I want, but
> > this is not explicitly mentioned anywhere and Linux uses the GNTTABOP
> > mechanism.
> >
> > Further to that, what is the format of the grant status frames?
> > grant_table.h doesn't have much to say about it.
> >
> > And lastly, given that I want the v2 grant ABI exclusively, I presume it's
> > sufficient to call GNTTABOP_set_version (version=2) first thing and abort
> > if it failed? Presumably the default is always v1 at start of day?
> 
> What kind of guests are you trying to target here?

PVHv2 only. x86_64 only for now, though the code should remain easily
portable to at least ARM64, should someone decide they need that.

> Since my reply, I tried to experiment, and I think you're forced to use
> GNTTABOP_setup_table/GNTTABOP_get_status_frames for x86 PV guests, and
> XENMEM_add_to_physmap for x86 HVM/PVH guests.
> 
> You can't depend on version 2 being available.  Its not available for
> ARM at all, and may be disabled for security reasons on x86 (there was
> some extended fun with transitive grants in the past, and we offered
> "totally disable grant v2" as one mitigation).

I don't need v2 at all, I was just going by the comments in grant_table.h,
which read: "Version 1 of the grant table entry structure is maintained
purely for backwards compatibility.  New guests should use version 2."

Grant status frames are a v2-only thing, right? Or can I use v1 and ask for
grant status frames?

-mato


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: XENMAPSPACE_grant_table vs. GNTTABOP_setup_table
  2020-06-10 13:56       ` Martin Lucina
@ 2020-06-10 14:21         ` Andrew Cooper
  2020-06-10 15:07           ` Martin Lucina
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Cooper @ 2020-06-10 14:21 UTC (permalink / raw)
  To: Martin Lucina, xen-devel, mirageos-devel

On 10/06/2020 14:56, Martin Lucina wrote:
> On Wednesday, 10.06.2020 at 14:40, Andrew Cooper wrote:
>>> So, going with the grant v2 ABI, is there a modern equivalent of
>>> GNTTABOP_get_status_frames? Reading memory.h I'm guessing that it might be
>>> XENMEM_add_to_physmap with space=XENMAPSPACE_grant_table and
>>> idx=(XENMAPIDX_grant_table_status + N) where N is the frame I want, but
>>> this is not explicitly mentioned anywhere and Linux uses the GNTTABOP
>>> mechanism.
>>>
>>> Further to that, what is the format of the grant status frames?
>>> grant_table.h doesn't have much to say about it.
>>>
>>> And lastly, given that I want the v2 grant ABI exclusively, I presume it's
>>> sufficient to call GNTTABOP_set_version (version=2) first thing and abort
>>> if it failed? Presumably the default is always v1 at start of day?
>> What kind of guests are you trying to target here?
> PVHv2 only. x86_64 only for now, though the code should remain easily
> portable to at least ARM64, should someone decide they need that.
>
>> Since my reply, I tried to experiment, and I think you're forced to use
>> GNTTABOP_setup_table/GNTTABOP_get_status_frames for x86 PV guests, and
>> XENMEM_add_to_physmap for x86 HVM/PVH guests.
>>
>> You can't depend on version 2 being available.  Its not available for
>> ARM at all, and may be disabled for security reasons on x86 (there was
>> some extended fun with transitive grants in the past, and we offered
>> "totally disable grant v2" as one mitigation).
> I don't need v2 at all, I was just going by the comments in grant_table.h,
> which read: "Version 1 of the grant table entry structure is maintained
> purely for backwards compatibility.  New guests should use version 2."

Ha...

That comment wasn't written with the benefit of hindsight.

IMO, grant v2 is not worth the astounding quantity of XSAs its
implementation actually gave us, or the complexity of the resulting code.

> Grant status frames are a v2-only thing, right?

Correct.

The split status frames was new in v2, in an attempt to reduce cacheline
ping-pong.

The downside is that the guest needs an unbounded loop trying to make a
change to the grant table while ensuring that the flags in the status
frame don't change asynchronously.

~Andrew

P.S. In some theoretical world, I was hoping to have something to live
in https://xenbits.xen.org/docs/latest/guest-guide/index.html on this
subject.  Do you mind if I use you as a retroactive guineapig if/when
some documentation were to appear?


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: XENMAPSPACE_grant_table vs. GNTTABOP_setup_table
  2020-06-10 14:21         ` Andrew Cooper
@ 2020-06-10 15:07           ` Martin Lucina
  0 siblings, 0 replies; 9+ messages in thread
From: Martin Lucina @ 2020-06-10 15:07 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: xen-devel, mirageos-devel

On Wednesday, 10.06.2020 at 15:21, Andrew Cooper wrote:
> > I don't need v2 at all, I was just going by the comments in grant_table.h,
> > which read: "Version 1 of the grant table entry structure is maintained
> > purely for backwards compatibility.  New guests should use version 2."
> 
> Ha...
> 
> That comment wasn't written with the benefit of hindsight.
> 
> IMO, grant v2 is not worth the astounding quantity of XSAs its
> implementation actually gave us, or the complexity of the resulting code.
> 
> > Grant status frames are a v2-only thing, right?
> 
> Correct.
> 
> The split status frames was new in v2, in an attempt to reduce cacheline
> ping-pong.
> 
> The downside is that the guest needs an unbounded loop trying to make a
> change to the grant table while ensuring that the flags in the status
> frame don't change asynchronously.
> 
> ~Andrew
> 
> P.S. In some theoretical world, I was hoping to have something to live
> in https://xenbits.xen.org/docs/latest/guest-guide/index.html on this
> subject.  Do you mind if I use you as a retroactive guineapig if/when
> some documentation were to appear?

No problem, sure, go for it.

-mato


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-06-10 15:07 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-09  9:44 XENMAPSPACE_grant_table vs. GNTTABOP_setup_table Martin Lucina
2020-06-09 10:22 ` Andrew Cooper
2020-06-09 10:50   ` Anil Madhavapeddy
2020-06-09 11:29     ` Julien Grall
2020-06-10 13:22   ` Martin Lucina
2020-06-10 13:40     ` Andrew Cooper
2020-06-10 13:56       ` Martin Lucina
2020-06-10 14:21         ` Andrew Cooper
2020-06-10 15:07           ` Martin Lucina

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.