[PATCH v2 0/6] gnttab: add per-domain controls

Roger Pau Monne posted 6 patches 2 years, 7 months ago
Failed in applying to current master (apply log)
docs/man/xl.cfg.5.pod.in             |  12 +++
docs/man/xl.conf.5.pod.in            |  14 +++
tools/console/Makefile               |   4 +-
tools/console/daemon/io.c            |  25 ++++--
tools/helpers/init-xenstore-domain.c |   1 +
tools/include/libxl.h                |  14 +++
tools/libs/light/libxl_create.c      |   6 ++
tools/libs/light/libxl_dm.c          |   2 +
tools/libs/light/libxl_dom.c         |   2 +-
tools/libs/light/libxl_types.idl     |   2 +
tools/ocaml/libs/xc/xenctrl.ml       |   2 +
tools/ocaml/libs/xc/xenctrl.mli      |   2 +
tools/ocaml/libs/xc/xenctrl_stubs.c  |  12 ++-
tools/xenstore/xenstored_core.h      |   2 +-
tools/xenstore/xenstored_domain.c    |  91 ++++++++++----------
tools/xenstore/xenstored_minios.c    |   4 +-
tools/xl/xl.c                        |  15 ++++
tools/xl/xl.h                        |   2 +
tools/xl/xl_parse.c                  |  13 +++
xen/arch/arm/domain_build.c          |   4 +
xen/arch/x86/setup.c                 |   2 +
xen/common/domain.c                  |   3 +-
xen/common/grant_table.c             | 122 ++++++++++++++++++++++++++-
xen/include/public/domctl.h          |  15 +++-
xen/include/xen/grant_table.h        |   5 +-
25 files changed, 310 insertions(+), 66 deletions(-)
[PATCH v2 0/6] gnttab: add per-domain controls
Posted by Roger Pau Monne 2 years, 7 months ago
Hello,

First patch on the series is a trivial change to xenconsoled in order to
use xenforeignmemory stable library in order to map the shared console
ring instead of the unstable libxc interface. It's reviewed and ready to
go in.

Patches 2 and 3 allow setting the host wide command line `gnttab` option
on a per domain basis. That means selecting the max allowed grant table
version and whether transitive grants are allowed.

The last 3 patches attempt to implement support for creating guests
without a grant table. This requires some changes to xenstored in order
to partially support guests without a valid ring interface, as the lack
of grant table will prevent C xenstored from mapping the shared ring.
Note this is not an issue for Ocaml xenstored, as it still uses the
foreign memory interface to map the shared ring, and thus won't notice
the lack of grant table support on the domain.

Thanks, Roger.

Roger Pau Monne (6):
  tools/console: use xenforeigmemory to map console ring
  gnttab: allow setting max version per-domain
  gnttab: allow per-domain control over transitive grants
  tools/xenstored: use atexit to close interfaces
  tools/xenstored: partially handle domains without a shared ring
  gnttab: allow disabling grant table per-domain

 docs/man/xl.cfg.5.pod.in             |  12 +++
 docs/man/xl.conf.5.pod.in            |  14 +++
 tools/console/Makefile               |   4 +-
 tools/console/daemon/io.c            |  25 ++++--
 tools/helpers/init-xenstore-domain.c |   1 +
 tools/include/libxl.h                |  14 +++
 tools/libs/light/libxl_create.c      |   6 ++
 tools/libs/light/libxl_dm.c          |   2 +
 tools/libs/light/libxl_dom.c         |   2 +-
 tools/libs/light/libxl_types.idl     |   2 +
 tools/ocaml/libs/xc/xenctrl.ml       |   2 +
 tools/ocaml/libs/xc/xenctrl.mli      |   2 +
 tools/ocaml/libs/xc/xenctrl_stubs.c  |  12 ++-
 tools/xenstore/xenstored_core.h      |   2 +-
 tools/xenstore/xenstored_domain.c    |  91 ++++++++++----------
 tools/xenstore/xenstored_minios.c    |   4 +-
 tools/xl/xl.c                        |  15 ++++
 tools/xl/xl.h                        |   2 +
 tools/xl/xl_parse.c                  |  13 +++
 xen/arch/arm/domain_build.c          |   4 +
 xen/arch/x86/setup.c                 |   2 +
 xen/common/domain.c                  |   3 +-
 xen/common/grant_table.c             | 122 ++++++++++++++++++++++++++-
 xen/include/public/domctl.h          |  15 +++-
 xen/include/xen/grant_table.h        |   5 +-
 25 files changed, 310 insertions(+), 66 deletions(-)

-- 
2.33.0


Re: [PATCH v2 0/6] gnttab: add per-domain controls
Posted by Julien Grall 2 years, 7 months ago

On 22/09/2021 13:21, Roger Pau Monne wrote:
> Hello,

Hi Roger,

> First patch on the series is a trivial change to xenconsoled in order to
> use xenforeignmemory stable library in order to map the shared console
> ring instead of the unstable libxc interface. It's reviewed and ready to
> go in.
> 
> Patches 2 and 3 allow setting the host wide command line `gnttab` option
> on a per domain basis. That means selecting the max allowed grant table
> version and whether transitive grants are allowed.
> 
> The last 3 patches attempt to implement support for creating guests
> without a grant table. This requires some changes to xenstored in order
> to partially support guests without a valid ring interface, as the lack
> of grant table will prevent C xenstored from mapping the shared ring.
> Note this is not an issue for Ocaml xenstored, as it still uses the
> foreign memory interface to map the shared ring, and thus won't notice
> the lack of grant table support on the domain.

I find a bit odd that the Xenstore support is conditional to whether 
grant table is available. Are you expecting domains with no grant table 
to have no PV drivers (including PV shutdown)?

Cheers,

-- 
Julien Grall

Re: [PATCH v2 0/6] gnttab: add per-domain controls
Posted by Roger Pau Monné 2 years, 7 months ago
On Wed, Sep 22, 2021 at 01:57:02PM +0500, Julien Grall wrote:
> 
> 
> On 22/09/2021 13:21, Roger Pau Monne wrote:
> > Hello,
> 
> Hi Roger,
> 
> > First patch on the series is a trivial change to xenconsoled in order to
> > use xenforeignmemory stable library in order to map the shared console
> > ring instead of the unstable libxc interface. It's reviewed and ready to
> > go in.
> > 
> > Patches 2 and 3 allow setting the host wide command line `gnttab` option
> > on a per domain basis. That means selecting the max allowed grant table
> > version and whether transitive grants are allowed.
> > 
> > The last 3 patches attempt to implement support for creating guests
> > without a grant table. This requires some changes to xenstored in order
> > to partially support guests without a valid ring interface, as the lack
> > of grant table will prevent C xenstored from mapping the shared ring.
> > Note this is not an issue for Ocaml xenstored, as it still uses the
> > foreign memory interface to map the shared ring, and thus won't notice
> > the lack of grant table support on the domain.
> 
> I find a bit odd that the Xenstore support is conditional to whether grant
> table is available. Are you expecting domains with no grant table to have no
> PV drivers (including PV shutdown)?

I don't really expect much, as having guests without grant table is a
developer option right now, if someone wants to make use of them for
any reason it would need some thought.

The other option would be my first proposal to restore foreign mapping
of the xenstore ring on that case:

https://lore.kernel.org/xen-devel/20210917154625.89315-6-roger.pau@citrix.com/

But it's also arguable that a guest not having a grant table should
also likely prevent foreign mapping attempts. Plus such foreign
mapping won't work from stubdomains.

I'm fine with dropping those patches if they turn out to be too
controversial, I think it's an interesting option to be able to
disable the grant table, but I don't have a full picture of how that
could be used in practice. Maybe others have and would be willing to
pick this up.

The xenstored patch is mostly so that I can boot guests without a
grant table using xl and test it's disabled using XTF.

Regards, Roger.

Re: [PATCH v2 0/6] gnttab: add per-domain controls
Posted by Julien Grall 2 years, 7 months ago
Hi Roger,

On 22/09/2021 14:39, Roger Pau Monné wrote:
> On Wed, Sep 22, 2021 at 01:57:02PM +0500, Julien Grall wrote:
>>
>>
>> On 22/09/2021 13:21, Roger Pau Monne wrote:
>>> Hello,
>>
>> Hi Roger,
>>
>>> First patch on the series is a trivial change to xenconsoled in order to
>>> use xenforeignmemory stable library in order to map the shared console
>>> ring instead of the unstable libxc interface. It's reviewed and ready to
>>> go in.
>>>
>>> Patches 2 and 3 allow setting the host wide command line `gnttab` option
>>> on a per domain basis. That means selecting the max allowed grant table
>>> version and whether transitive grants are allowed.
>>>
>>> The last 3 patches attempt to implement support for creating guests
>>> without a grant table. This requires some changes to xenstored in order
>>> to partially support guests without a valid ring interface, as the lack
>>> of grant table will prevent C xenstored from mapping the shared ring.
>>> Note this is not an issue for Ocaml xenstored, as it still uses the
>>> foreign memory interface to map the shared ring, and thus won't notice
>>> the lack of grant table support on the domain.
>>
>> I find a bit odd that the Xenstore support is conditional to whether grant
>> table is available. Are you expecting domains with no grant table to have no
>> PV drivers (including PV shutdown)?
> 
> I don't really expect much, as having guests without grant table is a
> developer option right now, if someone wants to make use of them for
> any reason it would need some thought.
> 
> The other option would be my first proposal to restore foreign mapping
> of the xenstore ring on that case:
> 
> https://lore.kernel.org/xen-devel/20210917154625.89315-6-roger.pau@citrix.com/
> 
> But it's also arguable that a guest not having a grant table should
> also likely prevent foreign mapping attempts. Plus such foreign
> mapping won't work from stubdomains.

There is another option: extend the acquire hypercall to allow xenstored 
domain to map the xenstore interface. This would require more work, but 
at least it would avoid the interesting dependency on the grant table.

> 
> I'm fine with dropping those patches if they turn out to be too
> controversial, I think it's an interesting option to be able to
> disable the grant table, but I don't have a full picture of how that
> could be used in practice. Maybe others have and would be willing to
> pick this up.

I think the current approach is probably OK as a developper option. 
However, we should at least document in the option that disabling the 
grant-table will also disable Xenstore (anything else?) support when 
using C Xenstored.

It might also be worth to clearly state in the doc that this is only 
intended for developer use and not supported.

Cheers,

-- 
Julien Grall

Re: [PATCH v2 0/6] gnttab: add per-domain controls
Posted by Roger Pau Monné 2 years, 7 months ago
On Thu, Sep 23, 2021 at 01:47:37PM +0500, Julien Grall wrote:
> Hi Roger,
> 
> On 22/09/2021 14:39, Roger Pau Monné wrote:
> > On Wed, Sep 22, 2021 at 01:57:02PM +0500, Julien Grall wrote:
> > > 
> > > 
> > > On 22/09/2021 13:21, Roger Pau Monne wrote:
> > > > Hello,
> > > 
> > > Hi Roger,
> > > 
> > > > First patch on the series is a trivial change to xenconsoled in order to
> > > > use xenforeignmemory stable library in order to map the shared console
> > > > ring instead of the unstable libxc interface. It's reviewed and ready to
> > > > go in.
> > > > 
> > > > Patches 2 and 3 allow setting the host wide command line `gnttab` option
> > > > on a per domain basis. That means selecting the max allowed grant table
> > > > version and whether transitive grants are allowed.
> > > > 
> > > > The last 3 patches attempt to implement support for creating guests
> > > > without a grant table. This requires some changes to xenstored in order
> > > > to partially support guests without a valid ring interface, as the lack
> > > > of grant table will prevent C xenstored from mapping the shared ring.
> > > > Note this is not an issue for Ocaml xenstored, as it still uses the
> > > > foreign memory interface to map the shared ring, and thus won't notice
> > > > the lack of grant table support on the domain.
> > > 
> > > I find a bit odd that the Xenstore support is conditional to whether grant
> > > table is available. Are you expecting domains with no grant table to have no
> > > PV drivers (including PV shutdown)?
> > 
> > I don't really expect much, as having guests without grant table is a
> > developer option right now, if someone wants to make use of them for
> > any reason it would need some thought.
> > 
> > The other option would be my first proposal to restore foreign mapping
> > of the xenstore ring on that case:
> > 
> > https://lore.kernel.org/xen-devel/20210917154625.89315-6-roger.pau@citrix.com/
> > 
> > But it's also arguable that a guest not having a grant table should
> > also likely prevent foreign mapping attempts. Plus such foreign
> > mapping won't work from stubdomains.
> 
> There is another option: extend the acquire hypercall to allow xenstored
> domain to map the xenstore interface. This would require more work, but at
> least it would avoid the interesting dependency on the grant table.

Xen isn't aware of the shared xenstore ring page currently, so that
would mean introducing more knowledge to the hypervisor that what's
strictly required IMO, as Xen has no business in knowing such details.

The grant table slot used by the xenstore shared page is just an
agreement at the toolstack level, but not known to the hypervisor so
far.

> > 
> > I'm fine with dropping those patches if they turn out to be too
> > controversial, I think it's an interesting option to be able to
> > disable the grant table, but I don't have a full picture of how that
> > could be used in practice. Maybe others have and would be willing to
> > pick this up.
> 
> I think the current approach is probably OK as a developper option. However,
> we should at least document in the option that disabling the grant-table
> will also disable Xenstore (anything else?) support when using C Xenstored.
> 
> It might also be worth to clearly state in the doc that this is only
> intended for developer use and not supported.

Sure, adding it to xl.cfg man page is likely the best place. Will do
when updating the patches.

Thanks, Roger.

Re: [PATCH v2 0/6] gnttab: add per-domain controls
Posted by Julien Grall 2 years, 7 months ago
Hi Roger,

On Thu, 23 Sep 2021, 16:20 Roger Pau Monné, <roger.pau@citrix.com> wrote:

> On Thu, Sep 23, 2021 at 01:47:37PM +0500, Julien Grall wrote:
> > Hi Roger,
> >
> > On 22/09/2021 14:39, Roger Pau Monné wrote:
> > > On Wed, Sep 22, 2021 at 01:57:02PM +0500, Julien Grall wrote:
> > > >
> > > >
> > > > On 22/09/2021 13:21, Roger Pau Monne wrote:
> > > > > Hello,
> > > >
> > > > Hi Roger,
> > > >
> > > > > First patch on the series is a trivial change to xenconsoled in
> order to
> > > > > use xenforeignmemory stable library in order to map the shared
> console
> > > > > ring instead of the unstable libxc interface. It's reviewed and
> ready to
> > > > > go in.
> > > > >
> > > > > Patches 2 and 3 allow setting the host wide command line `gnttab`
> option
> > > > > on a per domain basis. That means selecting the max allowed grant
> table
> > > > > version and whether transitive grants are allowed.
> > > > >
> > > > > The last 3 patches attempt to implement support for creating guests
> > > > > without a grant table. This requires some changes to xenstored in
> order
> > > > > to partially support guests without a valid ring interface, as the
> lack
> > > > > of grant table will prevent C xenstored from mapping the shared
> ring.
> > > > > Note this is not an issue for Ocaml xenstored, as it still uses the
> > > > > foreign memory interface to map the shared ring, and thus won't
> notice
> > > > > the lack of grant table support on the domain.
> > > >
> > > > I find a bit odd that the Xenstore support is conditional to whether
> grant
> > > > table is available. Are you expecting domains with no grant table to
> have no
> > > > PV drivers (including PV shutdown)?
> > >
> > > I don't really expect much, as having guests without grant table is a
> > > developer option right now, if someone wants to make use of them for
> > > any reason it would need some thought.
> > >
> > > The other option would be my first proposal to restore foreign mapping
> > > of the xenstore ring on that case:
> > >
> > >
> https://lore.kernel.org/xen-devel/20210917154625.89315-6-roger.pau@citrix.com/
> > >
> > > But it's also arguable that a guest not having a grant table should
> > > also likely prevent foreign mapping attempts. Plus such foreign
> > > mapping won't work from stubdomains.
> >
> > There is another option: extend the acquire hypercall to allow xenstored
> > domain to map the xenstore interface. This would require more work, but
> at
> > least it would avoid the interesting dependency on the grant table.
>
> Xen isn't aware of the shared xenstore ring page currently, so that
> would mean introducing more knowledge to the hypervisor that what's
> strictly required IMO, as Xen has no business in knowing such details.
>

Well Xen already knows the page for HVM/PVH because the guest retrieve it
through an HMV param.

We only miss (?) the PV part.


> The grant table slot used by the xenstore shared page is just an
> agreement at the toolstack level, but not known to the hypervisor so
> far.
>

Right, we need to find a different way to provide/map the shared page if
the grant table is not present.

To me the acquire hypercall is the best way to resolve it as Xen knows
whether the domain will run Xenstored (at least we used to have a flag) and
we can do the permission control easily.

Do you have another alternative?


> > >
> > > I'm fine with dropping those patches if they turn out to be too
> > > controversial, I think it's an interesting option to be able to
> > > disable the grant table, but I don't have a full picture of how that
> > > could be used in practice. Maybe others have and would be willing to
> > > pick this up.
> >
> > I think the current approach is probably OK as a developper option.
> However,
> > we should at least document in the option that disabling the grant-table
> > will also disable Xenstore (anything else?) support when using C
> Xenstored.
> >
> > It might also be worth to clearly state in the doc that this is only
> > intended for developer use and not supported.
>
> Sure, adding it to xl.cfg man page is likely the best place. Will do
> when updating the patches.
>
> Thanks, Roger.
>
Re: [PATCH v2 0/6] gnttab: add per-domain controls
Posted by Jan Beulich 2 years, 7 months ago
On 24.09.2021 04:30, Julien Grall wrote:
> Hi Roger,
> 
> On Thu, 23 Sep 2021, 16:20 Roger Pau Monné, <roger.pau@citrix.com> wrote:
> 
>> On Thu, Sep 23, 2021 at 01:47:37PM +0500, Julien Grall wrote:
>>> Hi Roger,
>>>
>>> On 22/09/2021 14:39, Roger Pau Monné wrote:
>>>> On Wed, Sep 22, 2021 at 01:57:02PM +0500, Julien Grall wrote:
>>>>>
>>>>>
>>>>> On 22/09/2021 13:21, Roger Pau Monne wrote:
>>>>>> Hello,
>>>>>
>>>>> Hi Roger,
>>>>>
>>>>>> First patch on the series is a trivial change to xenconsoled in
>> order to
>>>>>> use xenforeignmemory stable library in order to map the shared
>> console
>>>>>> ring instead of the unstable libxc interface. It's reviewed and
>> ready to
>>>>>> go in.
>>>>>>
>>>>>> Patches 2 and 3 allow setting the host wide command line `gnttab`
>> option
>>>>>> on a per domain basis. That means selecting the max allowed grant
>> table
>>>>>> version and whether transitive grants are allowed.
>>>>>>
>>>>>> The last 3 patches attempt to implement support for creating guests
>>>>>> without a grant table. This requires some changes to xenstored in
>> order
>>>>>> to partially support guests without a valid ring interface, as the
>> lack
>>>>>> of grant table will prevent C xenstored from mapping the shared
>> ring.
>>>>>> Note this is not an issue for Ocaml xenstored, as it still uses the
>>>>>> foreign memory interface to map the shared ring, and thus won't
>> notice
>>>>>> the lack of grant table support on the domain.
>>>>>
>>>>> I find a bit odd that the Xenstore support is conditional to whether
>> grant
>>>>> table is available. Are you expecting domains with no grant table to
>> have no
>>>>> PV drivers (including PV shutdown)?
>>>>
>>>> I don't really expect much, as having guests without grant table is a
>>>> developer option right now, if someone wants to make use of them for
>>>> any reason it would need some thought.
>>>>
>>>> The other option would be my first proposal to restore foreign mapping
>>>> of the xenstore ring on that case:
>>>>
>>>>
>> https://lore.kernel.org/xen-devel/20210917154625.89315-6-roger.pau@citrix.com/
>>>>
>>>> But it's also arguable that a guest not having a grant table should
>>>> also likely prevent foreign mapping attempts. Plus such foreign
>>>> mapping won't work from stubdomains.
>>>
>>> There is another option: extend the acquire hypercall to allow xenstored
>>> domain to map the xenstore interface. This would require more work, but
>> at
>>> least it would avoid the interesting dependency on the grant table.
>>
>> Xen isn't aware of the shared xenstore ring page currently, so that
>> would mean introducing more knowledge to the hypervisor that what's
>> strictly required IMO, as Xen has no business in knowing such details.
>>
> 
> Well Xen already knows the page for HVM/PVH because the guest retrieve it
> through an HMV param.

To be honest using this in such a way would feel like an abuse / layering
violation to me.

Jan


Re: [PATCH v2 0/6] gnttab: add per-domain controls
Posted by Julien Grall 2 years, 7 months ago
On Fri, 24 Sep 2021, 11:21 Jan Beulich, <jbeulich@suse.com> wrote:

> On 24.09.2021 04:30, Julien Grall wrote:
> > Hi Roger,
> >
> > On Thu, 23 Sep 2021, 16:20 Roger Pau Monné, <roger.pau@citrix.com>
> wrote:
> >
> >> On Thu, Sep 23, 2021 at 01:47:37PM +0500, Julien Grall wrote:
> >>> Hi Roger,
> >>>
> >>> On 22/09/2021 14:39, Roger Pau Monné wrote:
> >>>> On Wed, Sep 22, 2021 at 01:57:02PM +0500, Julien Grall wrote:
> >>>>>
> >>>>>
> >>>>> On 22/09/2021 13:21, Roger Pau Monne wrote:
> >>>>>> Hello,
> >>>>>
> >>>>> Hi Roger,
> >>>>>
> >>>>>> First patch on the series is a trivial change to xenconsoled in
> >> order to
> >>>>>> use xenforeignmemory stable library in order to map the shared
> >> console
> >>>>>> ring instead of the unstable libxc interface. It's reviewed and
> >> ready to
> >>>>>> go in.
> >>>>>>
> >>>>>> Patches 2 and 3 allow setting the host wide command line `gnttab`
> >> option
> >>>>>> on a per domain basis. That means selecting the max allowed grant
> >> table
> >>>>>> version and whether transitive grants are allowed.
> >>>>>>
> >>>>>> The last 3 patches attempt to implement support for creating guests
> >>>>>> without a grant table. This requires some changes to xenstored in
> >> order
> >>>>>> to partially support guests without a valid ring interface, as the
> >> lack
> >>>>>> of grant table will prevent C xenstored from mapping the shared
> >> ring.
> >>>>>> Note this is not an issue for Ocaml xenstored, as it still uses the
> >>>>>> foreign memory interface to map the shared ring, and thus won't
> >> notice
> >>>>>> the lack of grant table support on the domain.
> >>>>>
> >>>>> I find a bit odd that the Xenstore support is conditional to whether
> >> grant
> >>>>> table is available. Are you expecting domains with no grant table to
> >> have no
> >>>>> PV drivers (including PV shutdown)?
> >>>>
> >>>> I don't really expect much, as having guests without grant table is a
> >>>> developer option right now, if someone wants to make use of them for
> >>>> any reason it would need some thought.
> >>>>
> >>>> The other option would be my first proposal to restore foreign mapping
> >>>> of the xenstore ring on that case:
> >>>>
> >>>>
> >>
> https://lore.kernel.org/xen-devel/20210917154625.89315-6-roger.pau@citrix.com/
> >>>>
> >>>> But it's also arguable that a guest not having a grant table should
> >>>> also likely prevent foreign mapping attempts. Plus such foreign
> >>>> mapping won't work from stubdomains.
> >>>
> >>> There is another option: extend the acquire hypercall to allow
> xenstored
> >>> domain to map the xenstore interface. This would require more work, but
> >> at
> >>> least it would avoid the interesting dependency on the grant table.
> >>
> >> Xen isn't aware of the shared xenstore ring page currently, so that
> >> would mean introducing more knowledge to the hypervisor that what's
> >> strictly required IMO, as Xen has no business in knowing such details.
> >>
> >
> > Well Xen already knows the page for HVM/PVH because the guest retrieve it
> > through an HMV param.
>
> To be honest using this in such a way would feel like an abuse / layering
> violation to me.
>

I can see how it can be seen like this. Do you have a better suggestion to
be able to map mapping without the foreign mapping interface and the grant
table?

Jan
>
>
Re: [PATCH v2 0/6] gnttab: add per-domain controls
Posted by Jan Beulich 2 years, 7 months ago
On 24.09.2021 09:30, Julien Grall wrote:
> On Fri, 24 Sep 2021, 11:21 Jan Beulich, <jbeulich@suse.com> wrote:
>> On 24.09.2021 04:30, Julien Grall wrote:
>>> On Thu, 23 Sep 2021, 16:20 Roger Pau Monné, <roger.pau@citrix.com>
>> wrote:
>>>> On Thu, Sep 23, 2021 at 01:47:37PM +0500, Julien Grall wrote:
>>>>> On 22/09/2021 14:39, Roger Pau Monné wrote:
>>>>>> On Wed, Sep 22, 2021 at 01:57:02PM +0500, Julien Grall wrote:
>>>>>>> On 22/09/2021 13:21, Roger Pau Monne wrote:
>>>>>> But it's also arguable that a guest not having a grant table should
>>>>>> also likely prevent foreign mapping attempts. Plus such foreign
>>>>>> mapping won't work from stubdomains.
>>>>>
>>>>> There is another option: extend the acquire hypercall to allow
>> xenstored
>>>>> domain to map the xenstore interface. This would require more work, but
>>>> at
>>>>> least it would avoid the interesting dependency on the grant table.
>>>>
>>>> Xen isn't aware of the shared xenstore ring page currently, so that
>>>> would mean introducing more knowledge to the hypervisor that what's
>>>> strictly required IMO, as Xen has no business in knowing such details.
>>>>
>>>
>>> Well Xen already knows the page for HVM/PVH because the guest retrieve it
>>> through an HMV param.
>>
>> To be honest using this in such a way would feel like an abuse / layering
>> violation to me.
>>
> 
> I can see how it can be seen like this. Do you have a better suggestion to
> be able to map mapping without the foreign mapping interface and the grant
> table?

Well, as was mentioned, PV would need covering anyway. And I think just
like with grants the guest should consent with such foreign mappings
outside of the "can map everything anyway" category. Hence I think if
such a capability is indeed needed/wanted, it ought to be the guest to
announce this page to Xen.

Jan


Re: [PATCH v2 0/6] gnttab: add per-domain controls
Posted by Roger Pau Monné 2 years, 7 months ago
On Fri, Sep 24, 2021 at 07:30:51AM +0500, Julien Grall wrote:
> Hi Roger,
> 
> On Thu, 23 Sep 2021, 16:20 Roger Pau Monné, <roger.pau@citrix.com> wrote:
> 
> > On Thu, Sep 23, 2021 at 01:47:37PM +0500, Julien Grall wrote:
> > > Hi Roger,
> > >
> > > On 22/09/2021 14:39, Roger Pau Monné wrote:
> > > > On Wed, Sep 22, 2021 at 01:57:02PM +0500, Julien Grall wrote:
> > > > >
> > > > >
> > > > > On 22/09/2021 13:21, Roger Pau Monne wrote:
> > > > > > Hello,
> > > > >
> > > > > Hi Roger,
> > > > >
> > > > > > First patch on the series is a trivial change to xenconsoled in
> > order to
> > > > > > use xenforeignmemory stable library in order to map the shared
> > console
> > > > > > ring instead of the unstable libxc interface. It's reviewed and
> > ready to
> > > > > > go in.
> > > > > >
> > > > > > Patches 2 and 3 allow setting the host wide command line `gnttab`
> > option
> > > > > > on a per domain basis. That means selecting the max allowed grant
> > table
> > > > > > version and whether transitive grants are allowed.
> > > > > >
> > > > > > The last 3 patches attempt to implement support for creating guests
> > > > > > without a grant table. This requires some changes to xenstored in
> > order
> > > > > > to partially support guests without a valid ring interface, as the
> > lack
> > > > > > of grant table will prevent C xenstored from mapping the shared
> > ring.
> > > > > > Note this is not an issue for Ocaml xenstored, as it still uses the
> > > > > > foreign memory interface to map the shared ring, and thus won't
> > notice
> > > > > > the lack of grant table support on the domain.
> > > > >
> > > > > I find a bit odd that the Xenstore support is conditional to whether
> > grant
> > > > > table is available. Are you expecting domains with no grant table to
> > have no
> > > > > PV drivers (including PV shutdown)?
> > > >
> > > > I don't really expect much, as having guests without grant table is a
> > > > developer option right now, if someone wants to make use of them for
> > > > any reason it would need some thought.
> > > >
> > > > The other option would be my first proposal to restore foreign mapping
> > > > of the xenstore ring on that case:
> > > >
> > > >
> > https://lore.kernel.org/xen-devel/20210917154625.89315-6-roger.pau@citrix.com/
> > > >
> > > > But it's also arguable that a guest not having a grant table should
> > > > also likely prevent foreign mapping attempts. Plus such foreign
> > > > mapping won't work from stubdomains.
> > >
> > > There is another option: extend the acquire hypercall to allow xenstored
> > > domain to map the xenstore interface. This would require more work, but
> > at
> > > least it would avoid the interesting dependency on the grant table.
> >
> > Xen isn't aware of the shared xenstore ring page currently, so that
> > would mean introducing more knowledge to the hypervisor that what's
> > strictly required IMO, as Xen has no business in knowing such details.
> >
> 
> Well Xen already knows the page for HVM/PVH because the guest retrieve it
> through an HMV param.

HVM params has always been a bit weird IMO, as some are known and used
by Xen (like the identity page tables) while others (like the xenstore
or console ones) are merely used as a way to pass information from
the toolstack into the domain.

> We only miss (?) the PV part.

Right - ideally we should use the same mechanism for PV and HVM, which
would rule out he piggyback on HVM params.

> 
> > The grant table slot used by the xenstore shared page is just an
> > agreement at the toolstack level, but not known to the hypervisor so
> > far.
> >
> 
> Right, we need to find a different way to provide/map the shared page if
> the grant table is not present.
> 
> To me the acquire hypercall is the best way to resolve it as Xen knows
> whether the domain will run Xenstored (at least we used to have a flag) and
> we can do the permission control easily.
> 
> Do you have another alternative?

As said before, I didn't give much through about a practical use case
for this. My main focus where patches 1 and 2 (which sadly seem to be
shadowed by this no grant-table option) in order to have more fine
grained control over the grant table support on a per-domain basis.
The no grant-table mode was mostly the cherry on the cake.

I could see people using no grant-table also likely wanting
no-xenstore and no-console as to prevent mappings from other domains
into it's private memory altogether. Then using Argo or a similar no
memory sharing mechanism in order to interact with other entities. So
I think we shouldn't over engineer this xenstore usage without grant
tables, as it might not even be relevant as a use-case itself.

Using the acquire hypercall could be a solution, but I think we would
also need to introduce a new hypercall then in order for user space to
be able to tell Xen about resource areas, so that the xenstore address
can be provided to Xen without (ab)using HVM params.

Thanks, Roger.

Re: [PATCH v2 0/6] gnttab: add per-domain controls
Posted by Roger Pau Monné 2 years, 6 months ago
On Wed, Sep 22, 2021 at 10:21:17AM +0200, Roger Pau Monne wrote:
> Hello,
> 
> First patch on the series is a trivial change to xenconsoled in order to
> use xenforeignmemory stable library in order to map the shared console
> ring instead of the unstable libxc interface. It's reviewed and ready to
> go in.
> 
> Patches 2 and 3 allow setting the host wide command line `gnttab` option
> on a per domain basis. That means selecting the max allowed grant table
> version and whether transitive grants are allowed.
> 
> The last 3 patches attempt to implement support for creating guests
> without a grant table. This requires some changes to xenstored in order
> to partially support guests without a valid ring interface, as the lack
> of grant table will prevent C xenstored from mapping the shared ring.
> Note this is not an issue for Ocaml xenstored, as it still uses the
> foreign memory interface to map the shared ring, and thus won't notice
> the lack of grant table support on the domain.
> 
> Thanks, Roger.
> 
> Roger Pau Monne (6):
>   tools/console: use xenforeigmemory to map console ring
>   gnttab: allow setting max version per-domain
>   gnttab: allow per-domain control over transitive grants

Ping? The two patches above didn't get any review in either v1 or v2.

Patch #1 should be ready to go in AFAICT.

Thanks, Roger.

Re: [PATCH v2 0/6] gnttab: add per-domain controls
Posted by Christian Lindig 2 years, 6 months ago

On 11 Oct 2021, at 10:36, Roger Pau Monne <roger.pau@citrix.com<mailto:roger.pau@citrix.com>> wrote:

Ping? The two patches above didn't get any review in either v1 or v2.

Patch #1 should be ready to go in AFAICT.

Acked-by: Christian Lindig <christian.lindig@citrix.com<mailto:christian.lindig@citrix.com>>