Stabilising some tools only HVMOPs?

Stabilising some tools only HVMOPs?

[Wei Liu]

Hi all

Tools people are in the process of splitting libxenctrl into a set of
stable libraries. One of the proposed libraries is libxendevicemodel
which has a collection of APIs that can be used by device model.

Currently we use QEMU as reference to extract symbols and go through
them one by one. Along the way we discover QEMU is using some tools
only HVMOPs.

The list of tools only HVMOPs used by QEMU are:

  #define HVMOP_track_dirty_vram    6
  #define HVMOP_modified_memory    7
  #define HVMOP_set_mem_type    8
  #define HVMOP_inject_msi         16
  #define HVMOP_create_ioreq_server 17
  #define HVMOP_get_ioreq_server_info 18
  #define HVMOP_map_io_range_to_ioreq_server 19
  #define HVMOP_unmap_io_range_from_ioreq_server 20
  #define HVMOP_destroy_ioreq_server 21
  #define HVMOP_set_ioreq_server_state 22

I'm curious about the rationale for making them tools only in the
first place and what needs to be done to make them stable.

The option to build stable library APIs on top of unstable hypervisor
APIs is always there, but that looks suboptimal to me.

Wei.

Re: Stabilising some tools only HVMOPs?

[Ian Campbell]

On Wed, 2016-02-17 at 17:28 +0000, Wei Liu wrote:
> Hi all
> 
> Tools people are in the process of splitting libxenctrl into a set of
> stable libraries. One of the proposed libraries is libxendevicemodel
> which has a collection of APIs that can be used by device model.
> 
> Currently we use QEMU as reference to extract symbols and go through
> them one by one. Along the way we discover QEMU is using some tools
> only HVMOPs.
> 
> The list of tools only HVMOPs used by QEMU are:
> 
>   #define HVMOP_track_dirty_vram    6
>   #define HVMOP_modified_memory    7
>   #define HVMOP_set_mem_type    8
>   #define HVMOP_inject_msi         16
>   #define HVMOP_create_ioreq_server 17
>   #define HVMOP_get_ioreq_server_info 18
>   #define HVMOP_map_io_range_to_ioreq_server 19
>   #define HVMOP_unmap_io_range_from_ioreq_server 20
>   #define HVMOP_destroy_ioreq_server 21
>   #define HVMOP_set_ioreq_server_state 22
> 
> I'm curious about the rationale for making them tools only in the
> first place and what needs to be done to make them stable.

FWIW (IMHO, YMMV etc) it is becoming increasing incorrect to consider the
device model as "tools" in the face of disaggregation and support for
(nearly) arbitrary upstream QEMU versions etc.

> 
> The option to build stable library APIs on top of unstable hypervisor
> APIs is always there, but that looks suboptimal to me.
> 
> Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: Stabilising some tools only HVMOPs?

[Jan Beulich]

>>> On 18.02.16 at 11:24, <ian.campbell@citrix.com> wrote:
> On Wed, 2016-02-17 at 17:28 +0000, Wei Liu wrote:
>> The list of tools only HVMOPs used by QEMU are:
>> 
>>   #define HVMOP_track_dirty_vram    6
>>   #define HVMOP_modified_memory    7
>>   #define HVMOP_set_mem_type    8
>>   #define HVMOP_inject_msi         16
>>   #define HVMOP_create_ioreq_server 17
>>   #define HVMOP_get_ioreq_server_info 18
>>   #define HVMOP_map_io_range_to_ioreq_server 19
>>   #define HVMOP_unmap_io_range_from_ioreq_server 20
>>   #define HVMOP_destroy_ioreq_server 21
>>   #define HVMOP_set_ioreq_server_state 22
>> 
>> I'm curious about the rationale for making them tools only in the
>> first place and what needs to be done to make them stable.
> 
> FWIW (IMHO, YMMV etc) it is becoming increasing incorrect to consider the
> device model as "tools" in the face of disaggregation and support for
> (nearly) arbitrary upstream QEMU versions etc.

As just written in the other reply, it depends on what exactly
qemu uses: libxc interfaces are fine, since the "tools only"
aspect in the public headers is mainly to allow us to alter
structure layouts and alike. The "tools only" aspect there in
particular is not to preclude entities like qemu (indirectly)
invoking such operations - that's instead being dealt with by
permission checks.

I.e. as long a qemu doesn't define __XEN_TOOLS__ for its
building, I think we're fine.

Jan

Re: Stabilising some tools only HVMOPs?

[Wei Liu]

On Thu, Feb 18, 2016 at 03:37:06AM -0700, Jan Beulich wrote:
> >>> On 18.02.16 at 11:24, <ian.campbell@citrix.com> wrote:
> > On Wed, 2016-02-17 at 17:28 +0000, Wei Liu wrote:
> >> The list of tools only HVMOPs used by QEMU are:
> >> 
> >>   #define HVMOP_track_dirty_vram    6
> >>   #define HVMOP_modified_memory    7
> >>   #define HVMOP_set_mem_type    8
> >>   #define HVMOP_inject_msi         16
> >>   #define HVMOP_create_ioreq_server 17
> >>   #define HVMOP_get_ioreq_server_info 18
> >>   #define HVMOP_map_io_range_to_ioreq_server 19
> >>   #define HVMOP_unmap_io_range_from_ioreq_server 20
> >>   #define HVMOP_destroy_ioreq_server 21
> >>   #define HVMOP_set_ioreq_server_state 22
> >> 
> >> I'm curious about the rationale for making them tools only in the
> >> first place and what needs to be done to make them stable.
> > 
> > FWIW (IMHO, YMMV etc) it is becoming increasing incorrect to consider the
> > device model as "tools" in the face of disaggregation and support for
> > (nearly) arbitrary upstream QEMU versions etc.
> 
> As just written in the other reply, it depends on what exactly
> qemu uses: libxc interfaces are fine, since the "tools only"
> aspect in the public headers is mainly to allow us to alter
> structure layouts and alike. The "tools only" aspect there in
> particular is not to preclude entities like qemu (indirectly)
> invoking such operations - that's instead being dealt with by
> permission checks.
> 
> I.e. as long a qemu doesn't define __XEN_TOOLS__ for its
> building, I think we're fine.
> 

OK, so you're suggesting building stable APIs on top of unstable ones.

That's doable but undesirable. Once libxendevicemodel APIs are set in
stone they need to be supported in the long run. The underlying
hypervisor structure can change, but they still need to support the
upper layer one way or another. We may as well think hard now to get
things correct.

Wei.

> Jan
> 

Re: Stabilising some tools only HVMOPs?

[Ian Campbell]

On Thu, 2016-02-18 at 10:45 +0000, Wei Liu wrote:
> On Thu, Feb 18, 2016 at 03:37:06AM -0700, Jan Beulich wrote:
> > > > > On 18.02.16 at 11:24, <ian.campbell@citrix.com> wrote:
> > > On Wed, 2016-02-17 at 17:28 +0000, Wei Liu wrote:
> > > > The list of tools only HVMOPs used by QEMU are:
> > > > 
> > > >   #define HVMOP_track_dirty_vram    6
> > > >   #define HVMOP_modified_memory    7
> > > >   #define HVMOP_set_mem_type    8
> > > >   #define HVMOP_inject_msi         16
> > > >   #define HVMOP_create_ioreq_server 17
> > > >   #define HVMOP_get_ioreq_server_info 18
> > > >   #define HVMOP_map_io_range_to_ioreq_server 19
> > > >   #define HVMOP_unmap_io_range_from_ioreq_server 20
> > > >   #define HVMOP_destroy_ioreq_server 21
> > > >   #define HVMOP_set_ioreq_server_state 22
> > > > 
> > > > I'm curious about the rationale for making them tools only in the
> > > > first place and what needs to be done to make them stable.
> > > 
> > > FWIW (IMHO, YMMV etc) it is becoming increasing incorrect to consider
> > > the
> > > device model as "tools" in the face of disaggregation and support for
> > > (nearly) arbitrary upstream QEMU versions etc.
> > 
> > As just written in the other reply, it depends on what exactly
> > qemu uses: libxc interfaces are fine, since the "tools only"
> > aspect in the public headers is mainly to allow us to alter
> > structure layouts and alike. The "tools only" aspect there in
> > particular is not to preclude entities like qemu (indirectly)
> > invoking such operations - that's instead being dealt with by
> > permission checks.
> > 
> > I.e. as long a qemu doesn't define __XEN_TOOLS__ for its
> > building, I think we're fine.
> > 
> 
> OK, so you're suggesting building stable APIs on top of unstable ones.
> 
> That's doable but undesirable. Once libxendevicemodel APIs are set in
> stone they need to be supported in the long run. The underlying
> hypervisor structure can change, but they still need to support the
> upper layer one way or another. We may as well think hard now to get
> things correct.

FWIW I think it is important that any API/ABI stable interfaces are not
supplied as part of the otherwise unstable libxenctrl/libxenguest pair --
just to avoid any confusion or misunderstanding regarding what is or is not
considered stable.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: Stabilising some tools only HVMOPs?

[Wei Liu]

On Thu, Feb 18, 2016 at 10:53:00AM +0000, Ian Campbell wrote:
> On Thu, 2016-02-18 at 10:45 +0000, Wei Liu wrote:
> > On Thu, Feb 18, 2016 at 03:37:06AM -0700, Jan Beulich wrote:
> > > > > > On 18.02.16 at 11:24, <ian.campbell@citrix.com> wrote:
> > > > On Wed, 2016-02-17 at 17:28 +0000, Wei Liu wrote:
> > > > > The list of tools only HVMOPs used by QEMU are:
> > > > > 
> > > > >   #define HVMOP_track_dirty_vram    6
> > > > >   #define HVMOP_modified_memory    7
> > > > >   #define HVMOP_set_mem_type    8
> > > > >   #define HVMOP_inject_msi         16
> > > > >   #define HVMOP_create_ioreq_server 17
> > > > >   #define HVMOP_get_ioreq_server_info 18
> > > > >   #define HVMOP_map_io_range_to_ioreq_server 19
> > > > >   #define HVMOP_unmap_io_range_from_ioreq_server 20
> > > > >   #define HVMOP_destroy_ioreq_server 21
> > > > >   #define HVMOP_set_ioreq_server_state 22
> > > > > 
> > > > > I'm curious about the rationale for making them tools only in the
> > > > > first place and what needs to be done to make them stable.
> > > > 
> > > > FWIW (IMHO, YMMV etc) it is becoming increasing incorrect to consider
> > > > the
> > > > device model as "tools" in the face of disaggregation and support for
> > > > (nearly) arbitrary upstream QEMU versions etc.
> > > 
> > > As just written in the other reply, it depends on what exactly
> > > qemu uses: libxc interfaces are fine, since the "tools only"
> > > aspect in the public headers is mainly to allow us to alter
> > > structure layouts and alike. The "tools only" aspect there in
> > > particular is not to preclude entities like qemu (indirectly)
> > > invoking such operations - that's instead being dealt with by
> > > permission checks.
> > > 
> > > I.e. as long a qemu doesn't define __XEN_TOOLS__ for its
> > > building, I think we're fine.
> > > 
> > 
> > OK, so you're suggesting building stable APIs on top of unstable ones.
> > 
> > That's doable but undesirable. Once libxendevicemodel APIs are set in
> > stone they need to be supported in the long run. The underlying
> > hypervisor structure can change, but they still need to support the
> > upper layer one way or another. We may as well think hard now to get
> > things correct.
> 
> FWIW I think it is important that any API/ABI stable interfaces are not
> supplied as part of the otherwise unstable libxenctrl/libxenguest pair --
> just to avoid any confusion or misunderstanding regarding what is or is not
> considered stable.
> 

Agreed.  Whichever route we take, there will be no APIs left in
libxenctrl and libxenguest pair.

Wei.

> Ian.
> 

Re: Stabilising some tools only HVMOPs?

[Jan Beulich]

>>> On 18.02.16 at 11:45, <wei.liu2@citrix.com> wrote:
> On Thu, Feb 18, 2016 at 03:37:06AM -0700, Jan Beulich wrote:
>> >>> On 18.02.16 at 11:24, <ian.campbell@citrix.com> wrote:
>> > On Wed, 2016-02-17 at 17:28 +0000, Wei Liu wrote:
>> >> The list of tools only HVMOPs used by QEMU are:
>> >> 
>> >>   #define HVMOP_track_dirty_vram    6
>> >>   #define HVMOP_modified_memory    7
>> >>   #define HVMOP_set_mem_type    8
>> >>   #define HVMOP_inject_msi         16
>> >>   #define HVMOP_create_ioreq_server 17
>> >>   #define HVMOP_get_ioreq_server_info 18
>> >>   #define HVMOP_map_io_range_to_ioreq_server 19
>> >>   #define HVMOP_unmap_io_range_from_ioreq_server 20
>> >>   #define HVMOP_destroy_ioreq_server 21
>> >>   #define HVMOP_set_ioreq_server_state 22
>> >> 
>> >> I'm curious about the rationale for making them tools only in the
>> >> first place and what needs to be done to make them stable.
>> > 
>> > FWIW (IMHO, YMMV etc) it is becoming increasing incorrect to consider the
>> > device model as "tools" in the face of disaggregation and support for
>> > (nearly) arbitrary upstream QEMU versions etc.
>> 
>> As just written in the other reply, it depends on what exactly
>> qemu uses: libxc interfaces are fine, since the "tools only"
>> aspect in the public headers is mainly to allow us to alter
>> structure layouts and alike. The "tools only" aspect there in
>> particular is not to preclude entities like qemu (indirectly)
>> invoking such operations - that's instead being dealt with by
>> permission checks.
>> 
>> I.e. as long a qemu doesn't define __XEN_TOOLS__ for its
>> building, I think we're fine.
>> 
> 
> OK, so you're suggesting building stable APIs on top of unstable ones.

No, sorry, I'm not, after having read your and Ian's earlier replies.

Jan

Re: Stabilising some tools only HVMOPs?

[Jan Beulich]

>>> On 17.02.16 at 18:28, <wei.liu2@citrix.com> wrote:
> Hi all
> 
> Tools people are in the process of splitting libxenctrl into a set of
> stable libraries. One of the proposed libraries is libxendevicemodel
> which has a collection of APIs that can be used by device model.
> 
> Currently we use QEMU as reference to extract symbols and go through
> them one by one. Along the way we discover QEMU is using some tools
> only HVMOPs.
> 
> The list of tools only HVMOPs used by QEMU are:
> 
>   #define HVMOP_track_dirty_vram    6
>   #define HVMOP_modified_memory    7
>   #define HVMOP_set_mem_type    8
>   #define HVMOP_inject_msi         16
>   #define HVMOP_create_ioreq_server 17
>   #define HVMOP_get_ioreq_server_info 18
>   #define HVMOP_map_io_range_to_ioreq_server 19
>   #define HVMOP_unmap_io_range_from_ioreq_server 20
>   #define HVMOP_destroy_ioreq_server 21
>   #define HVMOP_set_ioreq_server_state 22

I've just grep-ed both qemu trees, and neither appears to directly
use any of these constants. So as long as qemu's use is solely
through libxc interfaces, I don't see an immediate issue.

> I'm curious about the rationale for making them tools only in the
> first place and what needs to be done to make them stable.

Qemu, in the original consideration, may also have been deemed
part of the tools.

> The option to build stable library APIs on top of unstable hypervisor
> APIs is always there, but that looks suboptimal to me.

Well, as long as we continue to tie libxc to the hypervisor version,
I think hiding versioning issues there would be fine.

Jan

Re: Stabilising some tools only HVMOPs?

[Wei Liu]

On Thu, Feb 18, 2016 at 03:31:49AM -0700, Jan Beulich wrote:
> >>> On 17.02.16 at 18:28, <wei.liu2@citrix.com> wrote:
> > Hi all
> > 
> > Tools people are in the process of splitting libxenctrl into a set of
> > stable libraries. One of the proposed libraries is libxendevicemodel
> > which has a collection of APIs that can be used by device model.
> > 
> > Currently we use QEMU as reference to extract symbols and go through
> > them one by one. Along the way we discover QEMU is using some tools
> > only HVMOPs.
> > 
> > The list of tools only HVMOPs used by QEMU are:
> > 
> >   #define HVMOP_track_dirty_vram    6
> >   #define HVMOP_modified_memory    7
> >   #define HVMOP_set_mem_type    8
> >   #define HVMOP_inject_msi         16
> >   #define HVMOP_create_ioreq_server 17
> >   #define HVMOP_get_ioreq_server_info 18
> >   #define HVMOP_map_io_range_to_ioreq_server 19
> >   #define HVMOP_unmap_io_range_from_ioreq_server 20
> >   #define HVMOP_destroy_ioreq_server 21
> >   #define HVMOP_set_ioreq_server_state 22
> 
> I've just grep-ed both qemu trees, and neither appears to directly
> use any of these constants. So as long as qemu's use is solely
> through libxc interfaces, I don't see an immediate issue.
> 
> > I'm curious about the rationale for making them tools only in the
> > first place and what needs to be done to make them stable.
> 
> Qemu, in the original consideration, may also have been deemed
> part of the tools.
> 
> > The option to build stable library APIs on top of unstable hypervisor
> > APIs is always there, but that looks suboptimal to me.
> 
> Well, as long as we continue to tie libxc to the hypervisor version,
> I think hiding versioning issues there would be fine.
> 

I think you missed the first part of my email -- we are trying to split
part of libxc out to make it stable.  Ian's reply in a sibling thread
has made clear the rationale behind this.  

Libxc is still tied to hypervisor, but the libraries that we split out
are stable.

Wei.

> Jan
> 

Re: Stabilising some tools only HVMOPs?

[Ian Campbell]

On Thu, 2016-02-18 at 03:31 -0700, Jan Beulich wrote:
> > > > On 17.02.16 at 18:28, <wei.liu2@citrix.com> wrote:
> > Hi all
> > 
> > Tools people are in the process of splitting libxenctrl into a set of
> > stable libraries. One of the proposed libraries is libxendevicemodel
> > which has a collection of APIs that can be used by device model.
> > 
> > Currently we use QEMU as reference to extract symbols and go through
> > them one by one. Along the way we discover QEMU is using some tools
> > only HVMOPs.
> > 
> > The list of tools only HVMOPs used by QEMU are:
> > 
> >   #define HVMOP_track_dirty_vram    6
> >   #define HVMOP_modified_memory    7
> >   #define HVMOP_set_mem_type    8
> >   #define HVMOP_inject_msi         16
> >   #define HVMOP_create_ioreq_server 17
> >   #define HVMOP_get_ioreq_server_info 18
> >   #define HVMOP_map_io_range_to_ioreq_server 19
> >   #define HVMOP_unmap_io_range_from_ioreq_server 20
> >   #define HVMOP_destroy_ioreq_server 21
> >   #define HVMOP_set_ioreq_server_state 22
> 
> I've just grep-ed both qemu trees, and neither appears to directly
> use any of these constants. So as long as qemu's use is solely
> through libxc interfaces, I don't see an immediate issue.

The point is that we want to stop QEMU using libxc and instead make it use
the proposed libxendevicemodel which will provide a stable interface to the
Xen functionality required by QEMU (like I recently did for evtchn, gnttab
and privcmd functionality).

At the moment distros need to rebuild their QEMU whenever Xen is upgraded
to link against a new libxc, which introduces an annoying lockstep for
them.

It also breaks e.g. Debian's arrangements which allow for multiple
tools+hypervisors to be installed and selecting tools to match the booted
hypervisor.

> > The option to build stable library APIs on top of unstable hypervisor
> > APIs is always there, but that looks suboptimal to me.
> 
> Well, as long as we continue to tie libxc to the hypervisor version,
> I think hiding versioning issues there would be fine.
> 
> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: Stabilising some tools only HVMOPs?

[Jan Beulich]

>>> On 18.02.16 at 11:44, <ian.campbell@citrix.com> wrote:
> On Thu, 2016-02-18 at 03:31 -0700, Jan Beulich wrote:
>> > > > On 17.02.16 at 18:28, <wei.liu2@citrix.com> wrote:
>> > Hi all
>> > 
>> > Tools people are in the process of splitting libxenctrl into a set of
>> > stable libraries. One of the proposed libraries is libxendevicemodel
>> > which has a collection of APIs that can be used by device model.
>> > 
>> > Currently we use QEMU as reference to extract symbols and go through
>> > them one by one. Along the way we discover QEMU is using some tools
>> > only HVMOPs.
>> > 
>> > The list of tools only HVMOPs used by QEMU are:
>> > 
>> >   #define HVMOP_track_dirty_vram    6
>> >   #define HVMOP_modified_memory    7
>> >   #define HVMOP_set_mem_type    8
>> >   #define HVMOP_inject_msi         16
>> >   #define HVMOP_create_ioreq_server 17
>> >   #define HVMOP_get_ioreq_server_info 18
>> >   #define HVMOP_map_io_range_to_ioreq_server 19
>> >   #define HVMOP_unmap_io_range_from_ioreq_server 20
>> >   #define HVMOP_destroy_ioreq_server 21
>> >   #define HVMOP_set_ioreq_server_state 22
>> 
>> I've just grep-ed both qemu trees, and neither appears to directly
>> use any of these constants. So as long as qemu's use is solely
>> through libxc interfaces, I don't see an immediate issue.
> 
> The point is that we want to stop QEMU using libxc and instead make it use
> the proposed libxendevicemodel which will provide a stable interface to the
> Xen functionality required by QEMU (like I recently did for evtchn, gnttab
> and privcmd functionality).

In that case I'm afraid we indeed need to make those interfaces
stable, by introducing a new group: Stuff that's stable but not to
be exposed to guests (albeit this non-exposure is of course only
an aid to people writing guest side code, to not tempt them to use
what won't work from inside a guest anyway, and hence isn't
strictly needed).

Jan

Re: Stabilising some tools only HVMOPs?

[Wei Liu]

On Thu, Feb 18, 2016 at 03:55:36AM -0700, Jan Beulich wrote:
> >>> On 18.02.16 at 11:44, <ian.campbell@citrix.com> wrote:
> > On Thu, 2016-02-18 at 03:31 -0700, Jan Beulich wrote:
> >> > > > On 17.02.16 at 18:28, <wei.liu2@citrix.com> wrote:
> >> > Hi all
> >> > 
> >> > Tools people are in the process of splitting libxenctrl into a set of
> >> > stable libraries. One of the proposed libraries is libxendevicemodel
> >> > which has a collection of APIs that can be used by device model.
> >> > 
> >> > Currently we use QEMU as reference to extract symbols and go through
> >> > them one by one. Along the way we discover QEMU is using some tools
> >> > only HVMOPs.
> >> > 
> >> > The list of tools only HVMOPs used by QEMU are:
> >> > 
> >> >   #define HVMOP_track_dirty_vram    6
> >> >   #define HVMOP_modified_memory    7
> >> >   #define HVMOP_set_mem_type    8
> >> >   #define HVMOP_inject_msi         16
> >> >   #define HVMOP_create_ioreq_server 17
> >> >   #define HVMOP_get_ioreq_server_info 18
> >> >   #define HVMOP_map_io_range_to_ioreq_server 19
> >> >   #define HVMOP_unmap_io_range_from_ioreq_server 20
> >> >   #define HVMOP_destroy_ioreq_server 21
> >> >   #define HVMOP_set_ioreq_server_state 22
> >> 
> >> I've just grep-ed both qemu trees, and neither appears to directly
> >> use any of these constants. So as long as qemu's use is solely
> >> through libxc interfaces, I don't see an immediate issue.
> > 
> > The point is that we want to stop QEMU using libxc and instead make it use
> > the proposed libxendevicemodel which will provide a stable interface to the
> > Xen functionality required by QEMU (like I recently did for evtchn, gnttab
> > and privcmd functionality).
> 
> In that case I'm afraid we indeed need to make those interfaces
> stable, by introducing a new group: Stuff that's stable but not to
> be exposed to guests (albeit this non-exposure is of course only
> an aid to people writing guest side code, to not tempt them to use
> what won't work from inside a guest anyway, and hence isn't
> strictly needed).
> 

Right. I think the misunderstanding stems from the fact that
__XEN_TOOLS__ conflates two aspects -- only used by tools and not stable.

I think we still need the first property (only used by tools) but not
the second.

Wei.

> Jan
> 

Re: Stabilising some tools only HVMOPs?

[Jan Beulich]

>>> On 18.02.16 at 11:59, <wei.liu2@citrix.com> wrote:
> On Thu, Feb 18, 2016 at 03:55:36AM -0700, Jan Beulich wrote:
>> >>> On 18.02.16 at 11:44, <ian.campbell@citrix.com> wrote:
>> > On Thu, 2016-02-18 at 03:31 -0700, Jan Beulich wrote:
>> >> > > > On 17.02.16 at 18:28, <wei.liu2@citrix.com> wrote:
>> >> > Hi all
>> >> > 
>> >> > Tools people are in the process of splitting libxenctrl into a set of
>> >> > stable libraries. One of the proposed libraries is libxendevicemodel
>> >> > which has a collection of APIs that can be used by device model.
>> >> > 
>> >> > Currently we use QEMU as reference to extract symbols and go through
>> >> > them one by one. Along the way we discover QEMU is using some tools
>> >> > only HVMOPs.
>> >> > 
>> >> > The list of tools only HVMOPs used by QEMU are:
>> >> > 
>> >> >   #define HVMOP_track_dirty_vram    6
>> >> >   #define HVMOP_modified_memory    7
>> >> >   #define HVMOP_set_mem_type    8
>> >> >   #define HVMOP_inject_msi         16
>> >> >   #define HVMOP_create_ioreq_server 17
>> >> >   #define HVMOP_get_ioreq_server_info 18
>> >> >   #define HVMOP_map_io_range_to_ioreq_server 19
>> >> >   #define HVMOP_unmap_io_range_from_ioreq_server 20
>> >> >   #define HVMOP_destroy_ioreq_server 21
>> >> >   #define HVMOP_set_ioreq_server_state 22
>> >> 
>> >> I've just grep-ed both qemu trees, and neither appears to directly
>> >> use any of these constants. So as long as qemu's use is solely
>> >> through libxc interfaces, I don't see an immediate issue.
>> > 
>> > The point is that we want to stop QEMU using libxc and instead make it use
>> > the proposed libxendevicemodel which will provide a stable interface to the
>> > Xen functionality required by QEMU (like I recently did for evtchn, gnttab
>> > and privcmd functionality).
>> 
>> In that case I'm afraid we indeed need to make those interfaces
>> stable, by introducing a new group: Stuff that's stable but not to
>> be exposed to guests (albeit this non-exposure is of course only
>> an aid to people writing guest side code, to not tempt them to use
>> what won't work from inside a guest anyway, and hence isn't
>> strictly needed).
>> 
> 
> Right. I think the misunderstanding stems from the fact that
> __XEN_TOOLS__ conflates two aspects -- only used by tools and not stable.
> 
> I think we still need the first property (only used by tools) but not
> the second.

I agree on both points.

Jan

Re: Stabilising some tools only HVMOPs?

[Wei Liu]

On Wed, Feb 17, 2016 at 05:28:08PM +0000, Wei Liu wrote:
> Hi all
> 
> Tools people are in the process of splitting libxenctrl into a set of
> stable libraries. One of the proposed libraries is libxendevicemodel
> which has a collection of APIs that can be used by device model.
> 
> Currently we use QEMU as reference to extract symbols and go through
> them one by one. Along the way we discover QEMU is using some tools
> only HVMOPs.
> 
> The list of tools only HVMOPs used by QEMU are:
> 
>   #define HVMOP_track_dirty_vram    6
>   #define HVMOP_modified_memory    7
>   #define HVMOP_set_mem_type    8
>   #define HVMOP_inject_msi         16
>   #define HVMOP_create_ioreq_server 17
>   #define HVMOP_get_ioreq_server_info 18
>   #define HVMOP_map_io_range_to_ioreq_server 19
>   #define HVMOP_unmap_io_range_from_ioreq_server 20
>   #define HVMOP_destroy_ioreq_server 21
>   #define HVMOP_set_ioreq_server_state 22
> 

I think we come to the conclusion that these HVMOPs should be made
stable. And to do so I'm going to introduce a __XEN_TOOLS_STABLE__ macro
for them to distinguish from __XEN_TOOLS__.  And then libxendevicemodel
will have -D__XEN_TOOLS_STABLE__  only.

Does this sound sufficient?

Wei.

Re: Stabilising some tools only HVMOPs?

[Ian Jackson]

Wei Liu writes ("Re: Stabilising some tools only HVMOPs?"):
> I think we come to the conclusion that these HVMOPs should be made
> stable. And to do so I'm going to introduce a __XEN_TOOLS_STABLE__ macro
> for them to distinguish from __XEN_TOOLS__.  And then libxendevicemodel
> will have -D__XEN_TOOLS_STABLE__  only.
> 
> Does this sound sufficient?

It would be better to rename -D__XEN_TOOLS__ too, to
-D__XEN_TOOLS_UNSTABLE.

Ian.

Re: Stabilising some tools only HVMOPs?

[Wei Liu]

On Thu, Feb 18, 2016 at 04:28:16PM +0000, Ian Jackson wrote:
> Wei Liu writes ("Re: Stabilising some tools only HVMOPs?"):
> > I think we come to the conclusion that these HVMOPs should be made
> > stable. And to do so I'm going to introduce a __XEN_TOOLS_STABLE__ macro
> > for them to distinguish from __XEN_TOOLS__.  And then libxendevicemodel
> > will have -D__XEN_TOOLS_STABLE__  only.
> > 
> > Does this sound sufficient?
> 
> It would be better to rename -D__XEN_TOOLS__ too, to
> -D__XEN_TOOLS_UNSTABLE.
> 

Fine by me, of course.

Wei.

> Ian.

Re: Stabilising some tools only HVMOPs?

[Jan Beulich]

>>> On 18.02.16 at 17:28, <Ian.Jackson@eu.citrix.com> wrote:
> Wei Liu writes ("Re: Stabilising some tools only HVMOPs?"):
>> I think we come to the conclusion that these HVMOPs should be made
>> stable. And to do so I'm going to introduce a __XEN_TOOLS_STABLE__ macro
>> for them to distinguish from __XEN_TOOLS__.  And then libxendevicemodel
>> will have -D__XEN_TOOLS_STABLE__  only.
>> 
>> Does this sound sufficient?
> 
> It would be better to rename -D__XEN_TOOLS__ too, to
> -D__XEN_TOOLS_UNSTABLE.

Even if a minor one, this will create a compatibility problem for
out of tree code including the headers: Their builds will all of
the sudden break, until they figure they need to go and
#define this new manifest symbol. Otoh maybe we would
actually like to break their builds this way, to make them aware
of the fact. In which case maybe __XEN_TOOLS__ should be
retained for the stable portions?

Jan

Re: Stabilising some tools only HVMOPs?

[Ian Jackson]

Jan Beulich writes ("Re: Stabilising some tools only HVMOPs?"):
> On 18.02.16 at 17:28, <Ian.Jackson@eu.citrix.com> wrote:
> > Wei Liu writes ("Re: Stabilising some tools only HVMOPs?"):
> >> I think we come to the conclusion that these HVMOPs should be made
> >> stable. And to do so I'm going to introduce a __XEN_TOOLS_STABLE__ macro
> >> for them to distinguish from __XEN_TOOLS__.  And then libxendevicemodel
> >> will have -D__XEN_TOOLS_STABLE__  only.
> >> 
> >> Does this sound sufficient?
> > 
> > It would be better to rename -D__XEN_TOOLS__ too, to
> > -D__XEN_TOOLS_UNSTABLE.
> 
> Even if a minor one, this will create a compatibility problem for
> out of tree code including the headers: Their builds will all of
> the sudden break, until they figure they need to go and
> #define this new manifest symbol. Otoh maybe we would
> actually like to break their builds this way, to make them aware
> of the fact.

We're talking here mostly about the unstable API, right ?  Well this
is a kind of instability :-).

> In which case maybe __XEN_TOOLS__ should be
> retained for the stable portions?

In principle that might be nice but actually the library split means
they have to change anyway.

Ian.

Re: Stabilising some tools only HVMOPs?

[Wei Liu]

On Thu, Feb 18, 2016 at 09:41:01AM -0700, Jan Beulich wrote:
> >>> On 18.02.16 at 17:28, <Ian.Jackson@eu.citrix.com> wrote:
> > Wei Liu writes ("Re: Stabilising some tools only HVMOPs?"):
> >> I think we come to the conclusion that these HVMOPs should be made
> >> stable. And to do so I'm going to introduce a __XEN_TOOLS_STABLE__ macro
> >> for them to distinguish from __XEN_TOOLS__.  And then libxendevicemodel
> >> will have -D__XEN_TOOLS_STABLE__  only.
> >> 
> >> Does this sound sufficient?
> > 
> > It would be better to rename -D__XEN_TOOLS__ too, to
> > -D__XEN_TOOLS_UNSTABLE.
> 
> Even if a minor one, this will create a compatibility problem for
> out of tree code including the headers: Their builds will all of
> the sudden break, until they figure they need to go and
> #define this new manifest symbol. Otoh maybe we would
> actually like to break their builds this way, to make them aware
> of the fact. In which case maybe __XEN_TOOLS__ should be
> retained for the stable portions?
> 

I think we should break their build but I also want to be a bit nicer.
So off the top of my head, we can have something like:

#if defined (__XEN_TOOLS__)
# error "NOTE: if you want to continue to build against unstable tools interface, use __XEN_TOOLS_UNSTABLE__ instead"
#endif

And place this in public headers that used to have __XEN_TOOLS__.

Wei.


> Jan
> 

Re: Stabilising some tools only HVMOPs?

[Ian Campbell]

On Thu, 2016-02-18 at 12:51 +0000, Wei Liu wrote:
> On Wed, Feb 17, 2016 at 05:28:08PM +0000, Wei Liu wrote:
> > Hi all
> > 
> > Tools people are in the process of splitting libxenctrl into a set of
> > stable libraries. One of the proposed libraries is libxendevicemodel
> > which has a collection of APIs that can be used by device model.
> > 
> > Currently we use QEMU as reference to extract symbols and go through
> > them one by one. Along the way we discover QEMU is using some tools
> > only HVMOPs.
> > 
> > The list of tools only HVMOPs used by QEMU are:
> > 
> >   #define HVMOP_track_dirty_vram    6
> >   #define HVMOP_modified_memory    7
> >   #define HVMOP_set_mem_type    8
> >   #define HVMOP_inject_msi         16
> >   #define HVMOP_create_ioreq_server 17
> >   #define HVMOP_get_ioreq_server_info 18
> >   #define HVMOP_map_io_range_to_ioreq_server 19
> >   #define HVMOP_unmap_io_range_from_ioreq_server 20
> >   #define HVMOP_destroy_ioreq_server 21
> >   #define HVMOP_set_ioreq_server_state 22
> > 
> 
> I think we come to the conclusion that these HVMOPs should be made
> stable. And to do so I'm going to introduce a __XEN_TOOLS_STABLE__ macro
> for them to distinguish from __XEN_TOOLS__.  And then libxendevicemodel
> will have -D__XEN_TOOLS_STABLE__  only.
> 
> Does this sound sufficient?

FWIW it sounds like a lot less faff than the direction I was thinking of
taking this! (moving them to a new DMOP hypercall)

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Domctl and physdevop for passthrough (Was: Re: Stabilising some tools only HVMOPs?)

[Wei Liu]

On Wed, Feb 17, 2016 at 05:28:08PM +0000, Wei Liu wrote:
> Hi all
> 
> Tools people are in the process of splitting libxenctrl into a set of
> stable libraries. One of the proposed libraries is libxendevicemodel
> which has a collection of APIs that can be used by device model.
> 
> Currently we use QEMU as reference to extract symbols and go through
> them one by one. Along the way we discover QEMU is using some tools
> only HVMOPs.
> 
> The list of tools only HVMOPs used by QEMU are:
> 
>   #define HVMOP_track_dirty_vram    6
>   #define HVMOP_modified_memory    7
>   #define HVMOP_set_mem_type    8
>   #define HVMOP_inject_msi         16
>   #define HVMOP_create_ioreq_server 17
>   #define HVMOP_get_ioreq_server_info 18
>   #define HVMOP_map_io_range_to_ioreq_server 19
>   #define HVMOP_unmap_io_range_from_ioreq_server 20
>   #define HVMOP_destroy_ioreq_server 21
>   #define HVMOP_set_ioreq_server_state 22
> 

In the process of ploughing through QEMU symbols, there are some domctls
and physdevops used to do  passthrough. To make passthrough APIs in
libxendevicemodel we need to stabilise them as well. Can I use the same
trick __XEN_TOOLS_STABLE__ here? If not, what would be the preferred way
of doing this?

PASSTHRU
`xc_domain_bind_pt_pci_irq`     `XEN_DOMCTL_bind_pt_irq`    
`xc_domain_ioport_mapping`      `XEN_DOMCTL_ioport_mapping` 
`xc_domain_memory_mapping`      `XEN_DOMCTL_memory_mapping` 
`xc_domain_unbind_msi_irq`      `XEN_DOMCTL_unbind_pt_irq`  
`xc_domain_unbind_pt_irq`       `XEN_DOMCTL_unbind_pt_irq`  
`xc_domain_update_msi_irq`      `XEN_DOMCTL_bind_pt_irq`    
`xc_physdev_map_pirq`           `PHYSDEVOP_map_pirq`        
`xc_physdev_map_pirq_msi`       `PHYSDEVOP_map_pirq`        
`xc_physdev_unmap_pirq`         `PHYSDEVOP_unmap_pirq`      


Wei.

Domctl and physdevop for passthrough (Was: Re: Stabilising some tools only HVMOPs?)

[Jan Beulich]

>>> On 19.02.16 at 17:05, <wei.liu2@citrix.com> wrote:
> On Wed, Feb 17, 2016 at 05:28:08PM +0000, Wei Liu wrote:
>> Hi all
>> 
>> Tools people are in the process of splitting libxenctrl into a set of
>> stable libraries. One of the proposed libraries is libxendevicemodel
>> which has a collection of APIs that can be used by device model.
>> 
>> Currently we use QEMU as reference to extract symbols and go through
>> them one by one. Along the way we discover QEMU is using some tools
>> only HVMOPs.
>> 
>> The list of tools only HVMOPs used by QEMU are:
>> 
>>   #define HVMOP_track_dirty_vram    6
>>   #define HVMOP_modified_memory    7
>>   #define HVMOP_set_mem_type    8
>>   #define HVMOP_inject_msi         16
>>   #define HVMOP_create_ioreq_server 17
>>   #define HVMOP_get_ioreq_server_info 18
>>   #define HVMOP_map_io_range_to_ioreq_server 19
>>   #define HVMOP_unmap_io_range_from_ioreq_server 20
>>   #define HVMOP_destroy_ioreq_server 21
>>   #define HVMOP_set_ioreq_server_state 22
>> 
> 
> In the process of ploughing through QEMU symbols, there are some domctls
> and physdevops used to do  passthrough. To make passthrough APIs in
> libxendevicemodel we need to stabilise them as well. Can I use the same
> trick __XEN_TOOLS_STABLE__ here? If not, what would be the preferred way
> of doing this?
> 
> PASSTHRU
> `xc_domain_bind_pt_pci_irq`     `XEN_DOMCTL_bind_pt_irq`    
> `xc_domain_ioport_mapping`      `XEN_DOMCTL_ioport_mapping` 
> `xc_domain_memory_mapping`      `XEN_DOMCTL_memory_mapping` 
> `xc_domain_unbind_msi_irq`      `XEN_DOMCTL_unbind_pt_irq`  
> `xc_domain_unbind_pt_irq`       `XEN_DOMCTL_unbind_pt_irq`  
> `xc_domain_update_msi_irq`      `XEN_DOMCTL_bind_pt_irq`    
> `xc_physdev_map_pirq`           `PHYSDEVOP_map_pirq`        
> `xc_physdev_map_pirq_msi`       `PHYSDEVOP_map_pirq`        
> `xc_physdev_unmap_pirq`         `PHYSDEVOP_unmap_pirq`      

Mechanically I would say yes, but anything here which is also on
the XSA-77 waiver list would first need removing there (with
proper auditing and, if necessary, fixing).

Jan

Re: Domctl and physdevop for passthrough (Was: Re: Stabilising some tools only HVMOPs?)

[Wei Liu]

On Mon, Feb 22, 2016 at 04:28:19AM -0700, Jan Beulich wrote:
> >>> On 19.02.16 at 17:05, <wei.liu2@citrix.com> wrote:
> > On Wed, Feb 17, 2016 at 05:28:08PM +0000, Wei Liu wrote:
> >> Hi all
> >> 
> >> Tools people are in the process of splitting libxenctrl into a set of
> >> stable libraries. One of the proposed libraries is libxendevicemodel
> >> which has a collection of APIs that can be used by device model.
> >> 
> >> Currently we use QEMU as reference to extract symbols and go through
> >> them one by one. Along the way we discover QEMU is using some tools
> >> only HVMOPs.
> >> 
> >> The list of tools only HVMOPs used by QEMU are:
> >> 
> >>   #define HVMOP_track_dirty_vram    6
> >>   #define HVMOP_modified_memory    7
> >>   #define HVMOP_set_mem_type    8
> >>   #define HVMOP_inject_msi         16
> >>   #define HVMOP_create_ioreq_server 17
> >>   #define HVMOP_get_ioreq_server_info 18
> >>   #define HVMOP_map_io_range_to_ioreq_server 19
> >>   #define HVMOP_unmap_io_range_from_ioreq_server 20
> >>   #define HVMOP_destroy_ioreq_server 21
> >>   #define HVMOP_set_ioreq_server_state 22
> >> 
> > 
> > In the process of ploughing through QEMU symbols, there are some domctls
> > and physdevops used to do  passthrough. To make passthrough APIs in
> > libxendevicemodel we need to stabilise them as well. Can I use the same
> > trick __XEN_TOOLS_STABLE__ here? If not, what would be the preferred way
> > of doing this?
> > 
> > PASSTHRU
> > `xc_domain_bind_pt_pci_irq`     `XEN_DOMCTL_bind_pt_irq`    
> > `xc_domain_ioport_mapping`      `XEN_DOMCTL_ioport_mapping` 
> > `xc_domain_memory_mapping`      `XEN_DOMCTL_memory_mapping` 
> > `xc_domain_unbind_msi_irq`      `XEN_DOMCTL_unbind_pt_irq`  
> > `xc_domain_unbind_pt_irq`       `XEN_DOMCTL_unbind_pt_irq`  
> > `xc_domain_update_msi_irq`      `XEN_DOMCTL_bind_pt_irq`    
> > `xc_physdev_map_pirq`           `PHYSDEVOP_map_pirq`        
> > `xc_physdev_map_pirq_msi`       `PHYSDEVOP_map_pirq`        
> > `xc_physdev_unmap_pirq`         `PHYSDEVOP_unmap_pirq`      
> 
> Mechanically I would say yes, but anything here which is also on
> the XSA-77 waiver list would first need removing there (with
> proper auditing and, if necessary, fixing).
> 

Ack. Thanks for your reply.

Wei.

> Jan
> 

Re: Domctl and physdevop for passthrough (Was: Re: Stabilising some tools only HVMOPs?)

[Wei Liu]

On Mon, Feb 22, 2016 at 04:28:19AM -0700, Jan Beulich wrote:
> >>> On 19.02.16 at 17:05, <wei.liu2@citrix.com> wrote:
> > On Wed, Feb 17, 2016 at 05:28:08PM +0000, Wei Liu wrote:
> >> Hi all
> >> 
> >> Tools people are in the process of splitting libxenctrl into a set of
> >> stable libraries. One of the proposed libraries is libxendevicemodel
> >> which has a collection of APIs that can be used by device model.
> >> 
> >> Currently we use QEMU as reference to extract symbols and go through
> >> them one by one. Along the way we discover QEMU is using some tools
> >> only HVMOPs.
> >> 
> >> The list of tools only HVMOPs used by QEMU are:
> >> 
> >>   #define HVMOP_track_dirty_vram    6
> >>   #define HVMOP_modified_memory    7
> >>   #define HVMOP_set_mem_type    8
> >>   #define HVMOP_inject_msi         16
> >>   #define HVMOP_create_ioreq_server 17
> >>   #define HVMOP_get_ioreq_server_info 18
> >>   #define HVMOP_map_io_range_to_ioreq_server 19
> >>   #define HVMOP_unmap_io_range_from_ioreq_server 20
> >>   #define HVMOP_destroy_ioreq_server 21
> >>   #define HVMOP_set_ioreq_server_state 22
> >> 
> > 
> > In the process of ploughing through QEMU symbols, there are some domctls
> > and physdevops used to do  passthrough. To make passthrough APIs in
> > libxendevicemodel we need to stabilise them as well. Can I use the same
> > trick __XEN_TOOLS_STABLE__ here? If not, what would be the preferred way
> > of doing this?
> > 
> > PASSTHRU
> > `xc_domain_bind_pt_pci_irq`     `XEN_DOMCTL_bind_pt_irq`    
> > `xc_domain_ioport_mapping`      `XEN_DOMCTL_ioport_mapping` 
> > `xc_domain_memory_mapping`      `XEN_DOMCTL_memory_mapping` 
> > `xc_domain_unbind_msi_irq`      `XEN_DOMCTL_unbind_pt_irq`  
> > `xc_domain_unbind_pt_irq`       `XEN_DOMCTL_unbind_pt_irq`  
> > `xc_domain_update_msi_irq`      `XEN_DOMCTL_bind_pt_irq`    
> > `xc_physdev_map_pirq`           `PHYSDEVOP_map_pirq`        
> > `xc_physdev_map_pirq_msi`       `PHYSDEVOP_map_pirq`        
> > `xc_physdev_unmap_pirq`         `PHYSDEVOP_unmap_pirq`      
> 
> Mechanically I would say yes, but anything here which is also on
> the XSA-77 waiver list would first need removing there (with
> proper auditing and, if necessary, fixing).
> 

I admit I failed to parse xsm-flask.txt and XSA-77 and its implication,
so let's take a concrete example instead.

Say, now I need to stabilise XEN_DOMCTL_pin_mem_cacheattr, which is on
the list of domctls listed in xsm-flask.txt (presumably that's the
waiver list you talked about).

You said "needs removing there", and xsm-flask.txt says "suops not
listed here are considered safe for disaggregation", so the implication
is that we need to make XEN_DOMCTL_pin_mem_cacheattr safe for
disaggregation in order to move it off the list. Is this correct?

And in order to make it safe for disaggregation, I need to add adequate
XSM checks for it. Is this correct?

Wei.

> Jan
> 

Re: Domctl and physdevop for passthrough (Was: Re: Stabilising some tools only HVMOPs?)

[Jan Beulich]

>>> On 23.02.16 at 15:31, <wei.liu2@citrix.com> wrote:
> On Mon, Feb 22, 2016 at 04:28:19AM -0700, Jan Beulich wrote:
>> >>> On 19.02.16 at 17:05, <wei.liu2@citrix.com> wrote:
>> > On Wed, Feb 17, 2016 at 05:28:08PM +0000, Wei Liu wrote:
>> >> Hi all
>> >> 
>> >> Tools people are in the process of splitting libxenctrl into a set of
>> >> stable libraries. One of the proposed libraries is libxendevicemodel
>> >> which has a collection of APIs that can be used by device model.
>> >> 
>> >> Currently we use QEMU as reference to extract symbols and go through
>> >> them one by one. Along the way we discover QEMU is using some tools
>> >> only HVMOPs.
>> >> 
>> >> The list of tools only HVMOPs used by QEMU are:
>> >> 
>> >>   #define HVMOP_track_dirty_vram    6
>> >>   #define HVMOP_modified_memory    7
>> >>   #define HVMOP_set_mem_type    8
>> >>   #define HVMOP_inject_msi         16
>> >>   #define HVMOP_create_ioreq_server 17
>> >>   #define HVMOP_get_ioreq_server_info 18
>> >>   #define HVMOP_map_io_range_to_ioreq_server 19
>> >>   #define HVMOP_unmap_io_range_from_ioreq_server 20
>> >>   #define HVMOP_destroy_ioreq_server 21
>> >>   #define HVMOP_set_ioreq_server_state 22
>> >> 
>> > 
>> > In the process of ploughing through QEMU symbols, there are some domctls
>> > and physdevops used to do  passthrough. To make passthrough APIs in
>> > libxendevicemodel we need to stabilise them as well. Can I use the same
>> > trick __XEN_TOOLS_STABLE__ here? If not, what would be the preferred way
>> > of doing this?
>> > 
>> > PASSTHRU
>> > `xc_domain_bind_pt_pci_irq`     `XEN_DOMCTL_bind_pt_irq`    
>> > `xc_domain_ioport_mapping`      `XEN_DOMCTL_ioport_mapping` 
>> > `xc_domain_memory_mapping`      `XEN_DOMCTL_memory_mapping` 
>> > `xc_domain_unbind_msi_irq`      `XEN_DOMCTL_unbind_pt_irq`  
>> > `xc_domain_unbind_pt_irq`       `XEN_DOMCTL_unbind_pt_irq`  
>> > `xc_domain_update_msi_irq`      `XEN_DOMCTL_bind_pt_irq`    
>> > `xc_physdev_map_pirq`           `PHYSDEVOP_map_pirq`        
>> > `xc_physdev_map_pirq_msi`       `PHYSDEVOP_map_pirq`        
>> > `xc_physdev_unmap_pirq`         `PHYSDEVOP_unmap_pirq`      
>> 
>> Mechanically I would say yes, but anything here which is also on
>> the XSA-77 waiver list would first need removing there (with
>> proper auditing and, if necessary, fixing).
>> 
> 
> I admit I failed to parse xsm-flask.txt and XSA-77 and its implication,
> so let's take a concrete example instead.
> 
> Say, now I need to stabilise XEN_DOMCTL_pin_mem_cacheattr, which is on
> the list of domctls listed in xsm-flask.txt (presumably that's the
> waiver list you talked about).
> 
> You said "needs removing there", and xsm-flask.txt says "suops not
> listed here are considered safe for disaggregation", so the implication
> is that we need to make XEN_DOMCTL_pin_mem_cacheattr safe for
> disaggregation in order to move it off the list. Is this correct?

Yes.

> And in order to make it safe for disaggregation, I need to add adequate
> XSM checks for it. Is this correct?

Well, that depends on what accessibility scope you mean to give
it: If domains other than the hardware and control domain are
meant to be permitted to access this with the dummy policy, then
it'll take more than XSM checks. In particular, it would then need
auditing to ensure that its use cannot compromise the system in
any way. For the particular example you gave, together with
XSA-154, I consider this close to impossible without perhaps
extensive code changes.

Jan

Re: Domctl and physdevop for passthrough (Was: Re: Stabilising some tools only HVMOPs?)

[Wei Liu]

On Tue, Feb 23, 2016 at 08:46:14AM -0700, Jan Beulich wrote:
> >>> On 23.02.16 at 15:31, <wei.liu2@citrix.com> wrote:
> > On Mon, Feb 22, 2016 at 04:28:19AM -0700, Jan Beulich wrote:
> >> >>> On 19.02.16 at 17:05, <wei.liu2@citrix.com> wrote:
> >> > On Wed, Feb 17, 2016 at 05:28:08PM +0000, Wei Liu wrote:
> >> >> Hi all
> >> >> 
> >> >> Tools people are in the process of splitting libxenctrl into a set of
> >> >> stable libraries. One of the proposed libraries is libxendevicemodel
> >> >> which has a collection of APIs that can be used by device model.
> >> >> 
> >> >> Currently we use QEMU as reference to extract symbols and go through
> >> >> them one by one. Along the way we discover QEMU is using some tools
> >> >> only HVMOPs.
> >> >> 
> >> >> The list of tools only HVMOPs used by QEMU are:
> >> >> 
> >> >>   #define HVMOP_track_dirty_vram    6
> >> >>   #define HVMOP_modified_memory    7
> >> >>   #define HVMOP_set_mem_type    8
> >> >>   #define HVMOP_inject_msi         16
> >> >>   #define HVMOP_create_ioreq_server 17
> >> >>   #define HVMOP_get_ioreq_server_info 18
> >> >>   #define HVMOP_map_io_range_to_ioreq_server 19
> >> >>   #define HVMOP_unmap_io_range_from_ioreq_server 20
> >> >>   #define HVMOP_destroy_ioreq_server 21
> >> >>   #define HVMOP_set_ioreq_server_state 22
> >> >> 
> >> > 
> >> > In the process of ploughing through QEMU symbols, there are some domctls
> >> > and physdevops used to do  passthrough. To make passthrough APIs in
> >> > libxendevicemodel we need to stabilise them as well. Can I use the same
> >> > trick __XEN_TOOLS_STABLE__ here? If not, what would be the preferred way
> >> > of doing this?
> >> > 
> >> > PASSTHRU
> >> > `xc_domain_bind_pt_pci_irq`     `XEN_DOMCTL_bind_pt_irq`    
> >> > `xc_domain_ioport_mapping`      `XEN_DOMCTL_ioport_mapping` 
> >> > `xc_domain_memory_mapping`      `XEN_DOMCTL_memory_mapping` 
> >> > `xc_domain_unbind_msi_irq`      `XEN_DOMCTL_unbind_pt_irq`  
> >> > `xc_domain_unbind_pt_irq`       `XEN_DOMCTL_unbind_pt_irq`  
> >> > `xc_domain_update_msi_irq`      `XEN_DOMCTL_bind_pt_irq`    
> >> > `xc_physdev_map_pirq`           `PHYSDEVOP_map_pirq`        
> >> > `xc_physdev_map_pirq_msi`       `PHYSDEVOP_map_pirq`        
> >> > `xc_physdev_unmap_pirq`         `PHYSDEVOP_unmap_pirq`      
> >> 
> >> Mechanically I would say yes, but anything here which is also on
> >> the XSA-77 waiver list would first need removing there (with
> >> proper auditing and, if necessary, fixing).
> >> 
> > 
> > I admit I failed to parse xsm-flask.txt and XSA-77 and its implication,
> > so let's take a concrete example instead.
> > 
> > Say, now I need to stabilise XEN_DOMCTL_pin_mem_cacheattr, which is on
> > the list of domctls listed in xsm-flask.txt (presumably that's the
> > waiver list you talked about).
> > 
> > You said "needs removing there", and xsm-flask.txt says "suops not
> > listed here are considered safe for disaggregation", so the implication
> > is that we need to make XEN_DOMCTL_pin_mem_cacheattr safe for
> > disaggregation in order to move it off the list. Is this correct?
> 
> Yes.
> 
> > And in order to make it safe for disaggregation, I need to add adequate
> > XSM checks for it. Is this correct?
> 
> Well, that depends on what accessibility scope you mean to give
> it: If domains other than the hardware and control domain are
> meant to be permitted to access this with the dummy policy, then

All the domctls and physdev ops are  going to used by device model. So
it is going to be either Dom0 or stub device model domain.

I do notice the following paragraph in xsm-flask.txt:

  This policy does not apply to bugs which affect stub device models,
  driver domains, or stub xenstored - even if those bugs do no worse
  than reduce the security of such a system to one whose device models,
  backend drivers, or xenstore, run in dom0.

Not sure how it changes the perspective.

> it'll take more than XSM checks. In particular, it would then need
> auditing to ensure that its use cannot compromise the system in
> any way. For the particular example you gave, together with
> XSA-154, I consider this close to impossible without perhaps
> extensive code changes.
> 

Indeed. This doesn't seem feasible.

If stabilising all these interfaces turn out extremely hard, I think I
can live with not stabilising them and change libxendevicemodel as they
change.

Wei.

> Jan
> 

Re: Domctl and physdevop for passthrough (Was: Re: Stabilising some tools only HVMOPs?)

[Jan Beulich]

>>> On 23.02.16 at 18:09, <wei.liu2@citrix.com> wrote:
> On Tue, Feb 23, 2016 at 08:46:14AM -0700, Jan Beulich wrote:
>> >>> On 23.02.16 at 15:31, <wei.liu2@citrix.com> wrote:
>> > On Mon, Feb 22, 2016 at 04:28:19AM -0700, Jan Beulich wrote:
>> >> >>> On 19.02.16 at 17:05, <wei.liu2@citrix.com> wrote:
>> >> > On Wed, Feb 17, 2016 at 05:28:08PM +0000, Wei Liu wrote:
>> >> >> Hi all
>> >> >> 
>> >> >> Tools people are in the process of splitting libxenctrl into a set of
>> >> >> stable libraries. One of the proposed libraries is libxendevicemodel
>> >> >> which has a collection of APIs that can be used by device model.
>> >> >> 
>> >> >> Currently we use QEMU as reference to extract symbols and go through
>> >> >> them one by one. Along the way we discover QEMU is using some tools
>> >> >> only HVMOPs.
>> >> >> 
>> >> >> The list of tools only HVMOPs used by QEMU are:
>> >> >> 
>> >> >>   #define HVMOP_track_dirty_vram    6
>> >> >>   #define HVMOP_modified_memory    7
>> >> >>   #define HVMOP_set_mem_type    8
>> >> >>   #define HVMOP_inject_msi         16
>> >> >>   #define HVMOP_create_ioreq_server 17
>> >> >>   #define HVMOP_get_ioreq_server_info 18
>> >> >>   #define HVMOP_map_io_range_to_ioreq_server 19
>> >> >>   #define HVMOP_unmap_io_range_from_ioreq_server 20
>> >> >>   #define HVMOP_destroy_ioreq_server 21
>> >> >>   #define HVMOP_set_ioreq_server_state 22
>> >> >> 
>> >> > 
>> >> > In the process of ploughing through QEMU symbols, there are some domctls
>> >> > and physdevops used to do  passthrough. To make passthrough APIs in
>> >> > libxendevicemodel we need to stabilise them as well. Can I use the same
>> >> > trick __XEN_TOOLS_STABLE__ here? If not, what would be the preferred way
>> >> > of doing this?
>> >> > 
>> >> > PASSTHRU
>> >> > `xc_domain_bind_pt_pci_irq`     `XEN_DOMCTL_bind_pt_irq`    
>> >> > `xc_domain_ioport_mapping`      `XEN_DOMCTL_ioport_mapping` 
>> >> > `xc_domain_memory_mapping`      `XEN_DOMCTL_memory_mapping` 
>> >> > `xc_domain_unbind_msi_irq`      `XEN_DOMCTL_unbind_pt_irq`  
>> >> > `xc_domain_unbind_pt_irq`       `XEN_DOMCTL_unbind_pt_irq`  
>> >> > `xc_domain_update_msi_irq`      `XEN_DOMCTL_bind_pt_irq`    
>> >> > `xc_physdev_map_pirq`           `PHYSDEVOP_map_pirq`        
>> >> > `xc_physdev_map_pirq_msi`       `PHYSDEVOP_map_pirq`        
>> >> > `xc_physdev_unmap_pirq`         `PHYSDEVOP_unmap_pirq`      
>> >> 
>> >> Mechanically I would say yes, but anything here which is also on
>> >> the XSA-77 waiver list would first need removing there (with
>> >> proper auditing and, if necessary, fixing).
>> >> 
>> > 
>> > I admit I failed to parse xsm-flask.txt and XSA-77 and its implication,
>> > so let's take a concrete example instead.
>> > 
>> > Say, now I need to stabilise XEN_DOMCTL_pin_mem_cacheattr, which is on
>> > the list of domctls listed in xsm-flask.txt (presumably that's the
>> > waiver list you talked about).
>> > 
>> > You said "needs removing there", and xsm-flask.txt says "suops not
>> > listed here are considered safe for disaggregation", so the implication
>> > is that we need to make XEN_DOMCTL_pin_mem_cacheattr safe for
>> > disaggregation in order to move it off the list. Is this correct?
>> 
>> Yes.
>> 
>> > And in order to make it safe for disaggregation, I need to add adequate
>> > XSM checks for it. Is this correct?
>> 
>> Well, that depends on what accessibility scope you mean to give
>> it: If domains other than the hardware and control domain are
>> meant to be permitted to access this with the dummy policy, then
> 
> All the domctls and physdev ops are  going to used by device model. So
> it is going to be either Dom0 or stub device model domain.

Right, but a stub domain needs to be treated as untrusted, so in
a way it's even worse than tool stack disaggregation.

> I do notice the following paragraph in xsm-flask.txt:
> 
>   This policy does not apply to bugs which affect stub device models,
>   driver domains, or stub xenstored - even if those bugs do no worse
>   than reduce the security of such a system to one whose device models,
>   backend drivers, or xenstore, run in dom0.
> 
> Not sure how it changes the perspective.

This tightens things (whereas I get the impression you view it as
relaxing them), in that issues in these interfaces which can be
exploited by any of the named entities would still be security
issues.

Jan

Re: Domctl and physdevop for passthrough (Was: Re: Stabilising some tools only HVMOPs?)

[Jan Beulich]

>>> On 23.02.16 at 18:24, <JBeulich@suse.com> wrote:
>>>> On 23.02.16 at 18:09, <wei.liu2@citrix.com> wrote:
>> On Tue, Feb 23, 2016 at 08:46:14AM -0700, Jan Beulich wrote:
>>> >>> On 23.02.16 at 15:31, <wei.liu2@citrix.com> wrote:
>>> > On Mon, Feb 22, 2016 at 04:28:19AM -0700, Jan Beulich wrote:
>>> >> >>> On 19.02.16 at 17:05, <wei.liu2@citrix.com> wrote:
>>> >> > On Wed, Feb 17, 2016 at 05:28:08PM +0000, Wei Liu wrote:
>>> >> >> Hi all
>>> >> >> 
>>> >> >> Tools people are in the process of splitting libxenctrl into a set of
>>> >> >> stable libraries. One of the proposed libraries is libxendevicemodel
>>> >> >> which has a collection of APIs that can be used by device model.
>>> >> >> 
>>> >> >> Currently we use QEMU as reference to extract symbols and go through
>>> >> >> them one by one. Along the way we discover QEMU is using some tools
>>> >> >> only HVMOPs.
>>> >> >> 
>>> >> >> The list of tools only HVMOPs used by QEMU are:
>>> >> >> 
>>> >> >>   #define HVMOP_track_dirty_vram    6
>>> >> >>   #define HVMOP_modified_memory    7
>>> >> >>   #define HVMOP_set_mem_type    8
>>> >> >>   #define HVMOP_inject_msi         16
>>> >> >>   #define HVMOP_create_ioreq_server 17
>>> >> >>   #define HVMOP_get_ioreq_server_info 18
>>> >> >>   #define HVMOP_map_io_range_to_ioreq_server 19
>>> >> >>   #define HVMOP_unmap_io_range_from_ioreq_server 20
>>> >> >>   #define HVMOP_destroy_ioreq_server 21
>>> >> >>   #define HVMOP_set_ioreq_server_state 22
>>> >> >> 
>>> >> > 
>>> >> > In the process of ploughing through QEMU symbols, there are some domctls
>>> >> > and physdevops used to do  passthrough. To make passthrough APIs in
>>> >> > libxendevicemodel we need to stabilise them as well. Can I use the same
>>> >> > trick __XEN_TOOLS_STABLE__ here? If not, what would be the preferred way
>>> >> > of doing this?
>>> >> > 
>>> >> > PASSTHRU
>>> >> > `xc_domain_bind_pt_pci_irq`     `XEN_DOMCTL_bind_pt_irq`    
>>> >> > `xc_domain_ioport_mapping`      `XEN_DOMCTL_ioport_mapping` 
>>> >> > `xc_domain_memory_mapping`      `XEN_DOMCTL_memory_mapping` 
>>> >> > `xc_domain_unbind_msi_irq`      `XEN_DOMCTL_unbind_pt_irq`  
>>> >> > `xc_domain_unbind_pt_irq`       `XEN_DOMCTL_unbind_pt_irq`  
>>> >> > `xc_domain_update_msi_irq`      `XEN_DOMCTL_bind_pt_irq`    
>>> >> > `xc_physdev_map_pirq`           `PHYSDEVOP_map_pirq`        
>>> >> > `xc_physdev_map_pirq_msi`       `PHYSDEVOP_map_pirq`        
>>> >> > `xc_physdev_unmap_pirq`         `PHYSDEVOP_unmap_pirq`      
>>> >> 
>>> >> Mechanically I would say yes, but anything here which is also on
>>> >> the XSA-77 waiver list would first need removing there (with
>>> >> proper auditing and, if necessary, fixing).
>>> >> 
>>> > 
>>> > I admit I failed to parse xsm-flask.txt and XSA-77 and its implication,
>>> > so let's take a concrete example instead.
>>> > 
>>> > Say, now I need to stabilise XEN_DOMCTL_pin_mem_cacheattr, which is on
>>> > the list of domctls listed in xsm-flask.txt (presumably that's the
>>> > waiver list you talked about).
>>> > 
>>> > You said "needs removing there", and xsm-flask.txt says "suops not
>>> > listed here are considered safe for disaggregation", so the implication
>>> > is that we need to make XEN_DOMCTL_pin_mem_cacheattr safe for
>>> > disaggregation in order to move it off the list. Is this correct?
>>> 
>>> Yes.
>>> 
>>> > And in order to make it safe for disaggregation, I need to add adequate
>>> > XSM checks for it. Is this correct?
>>> 
>>> Well, that depends on what accessibility scope you mean to give
>>> it: If domains other than the hardware and control domain are
>>> meant to be permitted to access this with the dummy policy, then
>> 
>> All the domctls and physdev ops are  going to used by device model. So
>> it is going to be either Dom0 or stub device model domain.
> 
> Right, but a stub domain needs to be treated as untrusted, so in
> a way it's even worse than tool stack disaggregation.
> 
>> I do notice the following paragraph in xsm-flask.txt:
>> 
>>   This policy does not apply to bugs which affect stub device models,
>>   driver domains, or stub xenstored - even if those bugs do no worse
>>   than reduce the security of such a system to one whose device models,
>>   backend drivers, or xenstore, run in dom0.
>> 
>> Not sure how it changes the perspective.
> 
> This tightens things (whereas I get the impression you view it as
> relaxing them), in that issues in these interfaces which can be
> exploited by any of the named entities would still be security
> issues.

And note how, using your example, xsm/dummy.h enforces XSM_PRIV
for XEN_DOMCTL_pin_mem_cacheattr.

Jan

Re: Domctl and physdevop for passthrough (Was: Re: Stabilising some tools only HVMOPs?)

[Wei Liu]

On Tue, Feb 23, 2016 at 10:24:50AM -0700, Jan Beulich wrote:
> >>> On 23.02.16 at 18:09, <wei.liu2@citrix.com> wrote:
> > On Tue, Feb 23, 2016 at 08:46:14AM -0700, Jan Beulich wrote:
> >> >>> On 23.02.16 at 15:31, <wei.liu2@citrix.com> wrote:
> >> > On Mon, Feb 22, 2016 at 04:28:19AM -0700, Jan Beulich wrote:
> >> >> >>> On 19.02.16 at 17:05, <wei.liu2@citrix.com> wrote:
> >> >> > On Wed, Feb 17, 2016 at 05:28:08PM +0000, Wei Liu wrote:
> >> >> >> Hi all
> >> >> >> 
> >> >> >> Tools people are in the process of splitting libxenctrl into a set of
> >> >> >> stable libraries. One of the proposed libraries is libxendevicemodel
> >> >> >> which has a collection of APIs that can be used by device model.
> >> >> >> 
> >> >> >> Currently we use QEMU as reference to extract symbols and go through
> >> >> >> them one by one. Along the way we discover QEMU is using some tools
> >> >> >> only HVMOPs.
> >> >> >> 
> >> >> >> The list of tools only HVMOPs used by QEMU are:
> >> >> >> 
> >> >> >>   #define HVMOP_track_dirty_vram    6
> >> >> >>   #define HVMOP_modified_memory    7
> >> >> >>   #define HVMOP_set_mem_type    8
> >> >> >>   #define HVMOP_inject_msi         16
> >> >> >>   #define HVMOP_create_ioreq_server 17
> >> >> >>   #define HVMOP_get_ioreq_server_info 18
> >> >> >>   #define HVMOP_map_io_range_to_ioreq_server 19
> >> >> >>   #define HVMOP_unmap_io_range_from_ioreq_server 20
> >> >> >>   #define HVMOP_destroy_ioreq_server 21
> >> >> >>   #define HVMOP_set_ioreq_server_state 22
> >> >> >> 
> >> >> > 
> >> >> > In the process of ploughing through QEMU symbols, there are some domctls
> >> >> > and physdevops used to do  passthrough. To make passthrough APIs in
> >> >> > libxendevicemodel we need to stabilise them as well. Can I use the same
> >> >> > trick __XEN_TOOLS_STABLE__ here? If not, what would be the preferred way
> >> >> > of doing this?
> >> >> > 
> >> >> > PASSTHRU
> >> >> > `xc_domain_bind_pt_pci_irq`     `XEN_DOMCTL_bind_pt_irq`    
> >> >> > `xc_domain_ioport_mapping`      `XEN_DOMCTL_ioport_mapping` 
> >> >> > `xc_domain_memory_mapping`      `XEN_DOMCTL_memory_mapping` 
> >> >> > `xc_domain_unbind_msi_irq`      `XEN_DOMCTL_unbind_pt_irq`  
> >> >> > `xc_domain_unbind_pt_irq`       `XEN_DOMCTL_unbind_pt_irq`  
> >> >> > `xc_domain_update_msi_irq`      `XEN_DOMCTL_bind_pt_irq`    
> >> >> > `xc_physdev_map_pirq`           `PHYSDEVOP_map_pirq`        
> >> >> > `xc_physdev_map_pirq_msi`       `PHYSDEVOP_map_pirq`        
> >> >> > `xc_physdev_unmap_pirq`         `PHYSDEVOP_unmap_pirq`      
> >> >> 
> >> >> Mechanically I would say yes, but anything here which is also on
> >> >> the XSA-77 waiver list would first need removing there (with
> >> >> proper auditing and, if necessary, fixing).
> >> >> 
> >> > 
> >> > I admit I failed to parse xsm-flask.txt and XSA-77 and its implication,
> >> > so let's take a concrete example instead.
> >> > 
> >> > Say, now I need to stabilise XEN_DOMCTL_pin_mem_cacheattr, which is on
> >> > the list of domctls listed in xsm-flask.txt (presumably that's the
> >> > waiver list you talked about).
> >> > 
> >> > You said "needs removing there", and xsm-flask.txt says "suops not
> >> > listed here are considered safe for disaggregation", so the implication
> >> > is that we need to make XEN_DOMCTL_pin_mem_cacheattr safe for
> >> > disaggregation in order to move it off the list. Is this correct?
> >> 
> >> Yes.
> >> 
> >> > And in order to make it safe for disaggregation, I need to add adequate
> >> > XSM checks for it. Is this correct?
> >> 
> >> Well, that depends on what accessibility scope you mean to give
> >> it: If domains other than the hardware and control domain are
> >> meant to be permitted to access this with the dummy policy, then
> > 
> > All the domctls and physdev ops are  going to used by device model. So
> > it is going to be either Dom0 or stub device model domain.
> 
> Right, but a stub domain needs to be treated as untrusted, so in
> a way it's even worse than tool stack disaggregation.
> 

Yes, I agree.

> > I do notice the following paragraph in xsm-flask.txt:
> > 
> >   This policy does not apply to bugs which affect stub device models,
> >   driver domains, or stub xenstored - even if those bugs do no worse
> >   than reduce the security of such a system to one whose device models,
> >   backend drivers, or xenstore, run in dom0.
> > 
> > Not sure how it changes the perspective.
> 
> This tightens things (whereas I get the impression you view it as
> relaxing them), in that issues in these interfaces which can be
> exploited by any of the named entities would still be security
> issues.
> 

Indeed. I was thinking that relaxes things and got very confused
(couldn't even convince myself). Your explanation makes more sense.

Wei.

> Jan

Re: Domctl and physdevop for passthrough (Was: Re: Stabilising some tools only HVMOPs?)

[Wei Liu]

On Tue, Feb 23, 2016 at 02:31:30PM +0000, Wei Liu wrote:
> On Mon, Feb 22, 2016 at 04:28:19AM -0700, Jan Beulich wrote:
> > >>> On 19.02.16 at 17:05, <wei.liu2@citrix.com> wrote:
> > > On Wed, Feb 17, 2016 at 05:28:08PM +0000, Wei Liu wrote:
> > >> Hi all
> > >> 
> > >> Tools people are in the process of splitting libxenctrl into a set of
> > >> stable libraries. One of the proposed libraries is libxendevicemodel
> > >> which has a collection of APIs that can be used by device model.
> > >> 
> > >> Currently we use QEMU as reference to extract symbols and go through
> > >> them one by one. Along the way we discover QEMU is using some tools
> > >> only HVMOPs.
> > >> 
> > >> The list of tools only HVMOPs used by QEMU are:
> > >> 
> > >>   #define HVMOP_track_dirty_vram    6
> > >>   #define HVMOP_modified_memory    7
> > >>   #define HVMOP_set_mem_type    8
> > >>   #define HVMOP_inject_msi         16
> > >>   #define HVMOP_create_ioreq_server 17
> > >>   #define HVMOP_get_ioreq_server_info 18
> > >>   #define HVMOP_map_io_range_to_ioreq_server 19
> > >>   #define HVMOP_unmap_io_range_from_ioreq_server 20
> > >>   #define HVMOP_destroy_ioreq_server 21
> > >>   #define HVMOP_set_ioreq_server_state 22
> > >> 
> > > 
> > > In the process of ploughing through QEMU symbols, there are some domctls
> > > and physdevops used to do  passthrough. To make passthrough APIs in
> > > libxendevicemodel we need to stabilise them as well. Can I use the same
> > > trick __XEN_TOOLS_STABLE__ here? If not, what would be the preferred way
> > > of doing this?
> > > 
> > > PASSTHRU
> > > `xc_domain_bind_pt_pci_irq`     `XEN_DOMCTL_bind_pt_irq`    
> > > `xc_domain_ioport_mapping`      `XEN_DOMCTL_ioport_mapping` 
> > > `xc_domain_memory_mapping`      `XEN_DOMCTL_memory_mapping` 
> > > `xc_domain_unbind_msi_irq`      `XEN_DOMCTL_unbind_pt_irq`  
> > > `xc_domain_unbind_pt_irq`       `XEN_DOMCTL_unbind_pt_irq`  
> > > `xc_domain_update_msi_irq`      `XEN_DOMCTL_bind_pt_irq`    
> > > `xc_physdev_map_pirq`           `PHYSDEVOP_map_pirq`        
> > > `xc_physdev_map_pirq_msi`       `PHYSDEVOP_map_pirq`        
> > > `xc_physdev_unmap_pirq`         `PHYSDEVOP_unmap_pirq`      
> > 
> > Mechanically I would say yes, but anything here which is also on
> > the XSA-77 waiver list would first need removing there (with
> > proper auditing and, if necessary, fixing).
> > 
> 
> I admit I failed to parse xsm-flask.txt and XSA-77 and its implication,
> so let's take a concrete example instead.
> 
> Say, now I need to stabilise XEN_DOMCTL_pin_mem_cacheattr, which is on

The conversation thus far has indicated stabilising this particular
hypercall is no go.

The higher order goal is actually pinning the memory cache attribute for
video ram. I was thinking to have a set of dedicated hypercalls for
video ram.

But then my reading of XSA-154 suggests that no untrusted entity should
be allowed to alter the caching attribute, so a set of restricted
hypercalls might not be feasible either. I would like to know if my
reading is correct.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: Domctl and physdevop for passthrough (Was: Re: Stabilising some tools only HVMOPs?)

[Jan Beulich]

>>> On 29.02.16 at 13:23, <wei.liu2@citrix.com> wrote:
> On Tue, Feb 23, 2016 at 02:31:30PM +0000, Wei Liu wrote:
>> On Mon, Feb 22, 2016 at 04:28:19AM -0700, Jan Beulich wrote:
>> > >>> On 19.02.16 at 17:05, <wei.liu2@citrix.com> wrote:
>> > > On Wed, Feb 17, 2016 at 05:28:08PM +0000, Wei Liu wrote:
>> > >> Hi all
>> > >> 
>> > >> Tools people are in the process of splitting libxenctrl into a set of
>> > >> stable libraries. One of the proposed libraries is libxendevicemodel
>> > >> which has a collection of APIs that can be used by device model.
>> > >> 
>> > >> Currently we use QEMU as reference to extract symbols and go through
>> > >> them one by one. Along the way we discover QEMU is using some tools
>> > >> only HVMOPs.
>> > >> 
>> > >> The list of tools only HVMOPs used by QEMU are:
>> > >> 
>> > >>   #define HVMOP_track_dirty_vram    6
>> > >>   #define HVMOP_modified_memory    7
>> > >>   #define HVMOP_set_mem_type    8
>> > >>   #define HVMOP_inject_msi         16
>> > >>   #define HVMOP_create_ioreq_server 17
>> > >>   #define HVMOP_get_ioreq_server_info 18
>> > >>   #define HVMOP_map_io_range_to_ioreq_server 19
>> > >>   #define HVMOP_unmap_io_range_from_ioreq_server 20
>> > >>   #define HVMOP_destroy_ioreq_server 21
>> > >>   #define HVMOP_set_ioreq_server_state 22
>> > >> 
>> > > 
>> > > In the process of ploughing through QEMU symbols, there are some domctls
>> > > and physdevops used to do  passthrough. To make passthrough APIs in
>> > > libxendevicemodel we need to stabilise them as well. Can I use the same
>> > > trick __XEN_TOOLS_STABLE__ here? If not, what would be the preferred way
>> > > of doing this?
>> > > 
>> > > PASSTHRU
>> > > `xc_domain_bind_pt_pci_irq`     `XEN_DOMCTL_bind_pt_irq`    
>> > > `xc_domain_ioport_mapping`      `XEN_DOMCTL_ioport_mapping` 
>> > > `xc_domain_memory_mapping`      `XEN_DOMCTL_memory_mapping` 
>> > > `xc_domain_unbind_msi_irq`      `XEN_DOMCTL_unbind_pt_irq`  
>> > > `xc_domain_unbind_pt_irq`       `XEN_DOMCTL_unbind_pt_irq`  
>> > > `xc_domain_update_msi_irq`      `XEN_DOMCTL_bind_pt_irq`    
>> > > `xc_physdev_map_pirq`           `PHYSDEVOP_map_pirq`        
>> > > `xc_physdev_map_pirq_msi`       `PHYSDEVOP_map_pirq`        
>> > > `xc_physdev_unmap_pirq`         `PHYSDEVOP_unmap_pirq`      
>> > 
>> > Mechanically I would say yes, but anything here which is also on
>> > the XSA-77 waiver list would first need removing there (with
>> > proper auditing and, if necessary, fixing).
>> > 
>> 
>> I admit I failed to parse xsm-flask.txt and XSA-77 and its implication,
>> so let's take a concrete example instead.
>> 
>> Say, now I need to stabilise XEN_DOMCTL_pin_mem_cacheattr, which is on
> 
> The conversation thus far has indicated stabilising this particular
> hypercall is no go.
> 
> The higher order goal is actually pinning the memory cache attribute for
> video ram. I was thinking to have a set of dedicated hypercalls for
> video ram.
> 
> But then my reading of XSA-154 suggests that no untrusted entity should
> be allowed to alter the caching attribute, so a set of restricted
> hypercalls might not be feasible either. I would like to know if my
> reading is correct.

Yes, your reading is mostly correct: Of course this can be
permitted eventually, but only after having made such a model
safe against abuse.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: Domctl and physdevop for passthrough (Was: Re: Stabilising some tools only HVMOPs?)

[Wei Liu]

On Mon, Feb 29, 2016 at 05:29:54AM -0700, Jan Beulich wrote:
> >>> On 29.02.16 at 13:23, <wei.liu2@citrix.com> wrote:
> > On Tue, Feb 23, 2016 at 02:31:30PM +0000, Wei Liu wrote:
> >> On Mon, Feb 22, 2016 at 04:28:19AM -0700, Jan Beulich wrote:
> >> > >>> On 19.02.16 at 17:05, <wei.liu2@citrix.com> wrote:
> >> > > On Wed, Feb 17, 2016 at 05:28:08PM +0000, Wei Liu wrote:
> >> > >> Hi all
> >> > >> 
> >> > >> Tools people are in the process of splitting libxenctrl into a set of
> >> > >> stable libraries. One of the proposed libraries is libxendevicemodel
> >> > >> which has a collection of APIs that can be used by device model.
> >> > >> 
> >> > >> Currently we use QEMU as reference to extract symbols and go through
> >> > >> them one by one. Along the way we discover QEMU is using some tools
> >> > >> only HVMOPs.
> >> > >> 
> >> > >> The list of tools only HVMOPs used by QEMU are:
> >> > >> 
> >> > >>   #define HVMOP_track_dirty_vram    6
> >> > >>   #define HVMOP_modified_memory    7
> >> > >>   #define HVMOP_set_mem_type    8
> >> > >>   #define HVMOP_inject_msi         16
> >> > >>   #define HVMOP_create_ioreq_server 17
> >> > >>   #define HVMOP_get_ioreq_server_info 18
> >> > >>   #define HVMOP_map_io_range_to_ioreq_server 19
> >> > >>   #define HVMOP_unmap_io_range_from_ioreq_server 20
> >> > >>   #define HVMOP_destroy_ioreq_server 21
> >> > >>   #define HVMOP_set_ioreq_server_state 22
> >> > >> 
> >> > > 
> >> > > In the process of ploughing through QEMU symbols, there are some domctls
> >> > > and physdevops used to do  passthrough. To make passthrough APIs in
> >> > > libxendevicemodel we need to stabilise them as well. Can I use the same
> >> > > trick __XEN_TOOLS_STABLE__ here? If not, what would be the preferred way
> >> > > of doing this?
> >> > > 
> >> > > PASSTHRU
> >> > > `xc_domain_bind_pt_pci_irq`     `XEN_DOMCTL_bind_pt_irq`    
> >> > > `xc_domain_ioport_mapping`      `XEN_DOMCTL_ioport_mapping` 
> >> > > `xc_domain_memory_mapping`      `XEN_DOMCTL_memory_mapping` 
> >> > > `xc_domain_unbind_msi_irq`      `XEN_DOMCTL_unbind_pt_irq`  
> >> > > `xc_domain_unbind_pt_irq`       `XEN_DOMCTL_unbind_pt_irq`  
> >> > > `xc_domain_update_msi_irq`      `XEN_DOMCTL_bind_pt_irq`    
> >> > > `xc_physdev_map_pirq`           `PHYSDEVOP_map_pirq`        
> >> > > `xc_physdev_map_pirq_msi`       `PHYSDEVOP_map_pirq`        
> >> > > `xc_physdev_unmap_pirq`         `PHYSDEVOP_unmap_pirq`      
> >> > 
> >> > Mechanically I would say yes, but anything here which is also on
> >> > the XSA-77 waiver list would first need removing there (with
> >> > proper auditing and, if necessary, fixing).
> >> > 
> >> 
> >> I admit I failed to parse xsm-flask.txt and XSA-77 and its implication,
> >> so let's take a concrete example instead.
> >> 
> >> Say, now I need to stabilise XEN_DOMCTL_pin_mem_cacheattr, which is on
> > 
> > The conversation thus far has indicated stabilising this particular
> > hypercall is no go.
> > 
> > The higher order goal is actually pinning the memory cache attribute for
> > video ram. I was thinking to have a set of dedicated hypercalls for
> > video ram.
> > 
> > But then my reading of XSA-154 suggests that no untrusted entity should
> > be allowed to alter the caching attribute, so a set of restricted
> > hypercalls might not be feasible either. I would like to know if my
> > reading is correct.
> 
> Yes, your reading is mostly correct: Of course this can be
> permitted eventually, but only after having made such a model
> safe against abuse.
> 

I read the XSA-154 patch and think a little bit on whether making
dedicated hypercall is feasible.

1. The patch for XSA-154 mentions that only MMIO mappings with
   inconsistent attributes can cause system instability.
2. PV case is hard, but the device model library is only of interest to
   HVM domain, so PV can be ignored.
3. We want to continue honoring pinned cachability attributes for HVM
   domain.

It seems we have a way forward. Say, we have new hypercall just for
pinning video ram cachability attribute.

The new hypercall has following properties:

1. It can only be used on HVM domains.
2. It can only be used on mfns that are not in MMIO ranges, because
   vram is just normal ram.
3. It can only set the cachability attribute to WC (used by video ram).
4. It is not considered stable.

so that it won't be abused to change cachability attributes of MMIO
mappings on PV guest to make the host unstable. The stale data issue is
of no relevance as stated in XSA-154 patch.

Does this sound plausible?

Wei.

> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: Domctl and physdevop for passthrough (Was: Re: Stabilising some tools only HVMOPs?)

[Jan Beulich]

>>> On 29.02.16 at 19:12, <wei.liu2@citrix.com> wrote:
> I read the XSA-154 patch and think a little bit on whether making
> dedicated hypercall is feasible.
> 
> 1. The patch for XSA-154 mentions that only MMIO mappings with
>    inconsistent attributes can cause system instability.
> 2. PV case is hard, but the device model library is only of interest to
>    HVM domain, so PV can be ignored.
> 3. We want to continue honoring pinned cachability attributes for HVM
>    domain.
> 
> It seems we have a way forward. Say, we have new hypercall just for
> pinning video ram cachability attribute.
> 
> The new hypercall has following properties:
> 
> 1. It can only be used on HVM domains.
> 2. It can only be used on mfns that are not in MMIO ranges, because
>    vram is just normal ram.
> 3. It can only set the cachability attribute to WC (used by video ram).
> 4. It is not considered stable.
> 
> so that it won't be abused to change cachability attributes of MMIO
> mappings on PV guest to make the host unstable. The stale data issue is
> of no relevance as stated in XSA-154 patch.
> 
> Does this sound plausible?

Yes, it does, but it extends our dependency on what we've been
told in the context of XSA-154 is actually true (and has been true
for all earlier processor generations, and will continue to be true
in the future). But then I don't immediately see why the existing
pinning operation won't suffice: It's a domctl (i.e. we can change
it), you say you don't need it to be stable, and it's already
documented as being intended for RAM only (albeit iirc that's not
getting enforced anywhere right now). The main present
problem (which I don't see a new hypercall to solve) is that it's
GFN-based, and the GFN->MFN mapping can change after such
pinning got established. Otoh I think that by changing the
placement of the hvm_get_mem_pinned_cacheattr() calls we
could enforce the RAM-only aspect quite easily. Let me put
together a patch ...

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: Domctl and physdevop for passthrough (Was: Re: Stabilising some tools only HVMOPs?)

[Wei Liu]

On Tue, Mar 01, 2016 at 12:54:09AM -0700, Jan Beulich wrote:
> >>> On 29.02.16 at 19:12, <wei.liu2@citrix.com> wrote:
> > I read the XSA-154 patch and think a little bit on whether making
> > dedicated hypercall is feasible.
> > 
> > 1. The patch for XSA-154 mentions that only MMIO mappings with
> >    inconsistent attributes can cause system instability.
> > 2. PV case is hard, but the device model library is only of interest to
> >    HVM domain, so PV can be ignored.
> > 3. We want to continue honoring pinned cachability attributes for HVM
> >    domain.
> > 
> > It seems we have a way forward. Say, we have new hypercall just for
> > pinning video ram cachability attribute.
> > 
> > The new hypercall has following properties:
> > 
> > 1. It can only be used on HVM domains.
> > 2. It can only be used on mfns that are not in MMIO ranges, because
> >    vram is just normal ram.
> > 3. It can only set the cachability attribute to WC (used by video ram).
> > 4. It is not considered stable.
> > 
> > so that it won't be abused to change cachability attributes of MMIO
> > mappings on PV guest to make the host unstable. The stale data issue is
> > of no relevance as stated in XSA-154 patch.
> > 
> > Does this sound plausible?
> 
> Yes, it does, but it extends our dependency on what we've been
> told in the context of XSA-154 is actually true (and has been true
> for all earlier processor generations, and will continue to be true
> in the future).
> But then I don't immediately see why the existing
> pinning operation won't suffice: It's a domctl (i.e. we can change
> it), you say you don't need it to be stable, and it's already
> documented as being intended for RAM only (albeit iirc that's not
> getting enforced anywhere right now). The main present
> problem (which I don't see a new hypercall to solve) is that it's
> GFN-based, and the GFN->MFN mapping can change after such
> pinning got established. Otoh I think that by changing the
> placement of the hvm_get_mem_pinned_cacheattr() calls we
> could enforce the RAM-only aspect quite easily. Let me put
> together a patch ...
> 

That would be good. Thank you very much.

Wei.


> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: Domctl and physdevop for passthrough (Was: Re: Stabilising some tools only HVMOPs?)

[Jan Beulich]

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

>>> On 01.03.16 at 11:52, <wei.liu2@citrix.com> wrote:
> On Tue, Mar 01, 2016 at 12:54:09AM -0700, Jan Beulich wrote:
>> >>> On 29.02.16 at 19:12, <wei.liu2@citrix.com> wrote:
>> > I read the XSA-154 patch and think a little bit on whether making
>> > dedicated hypercall is feasible.
>> > 
>> > 1. The patch for XSA-154 mentions that only MMIO mappings with
>> >    inconsistent attributes can cause system instability.
>> > 2. PV case is hard, but the device model library is only of interest to
>> >    HVM domain, so PV can be ignored.
>> > 3. We want to continue honoring pinned cachability attributes for HVM
>> >    domain.
>> > 
>> > It seems we have a way forward. Say, we have new hypercall just for
>> > pinning video ram cachability attribute.
>> > 
>> > The new hypercall has following properties:
>> > 
>> > 1. It can only be used on HVM domains.
>> > 2. It can only be used on mfns that are not in MMIO ranges, because
>> >    vram is just normal ram.
>> > 3. It can only set the cachability attribute to WC (used by video ram).
>> > 4. It is not considered stable.
>> > 
>> > so that it won't be abused to change cachability attributes of MMIO
>> > mappings on PV guest to make the host unstable. The stale data issue is
>> > of no relevance as stated in XSA-154 patch.
>> > 
>> > Does this sound plausible?
>> 
>> Yes, it does, but it extends our dependency on what we've been
>> told in the context of XSA-154 is actually true (and has been true
>> for all earlier processor generations, and will continue to be true
>> in the future).
>> But then I don't immediately see why the existing
>> pinning operation won't suffice: It's a domctl (i.e. we can change
>> it), you say you don't need it to be stable, and it's already
>> documented as being intended for RAM only (albeit iirc that's not
>> getting enforced anywhere right now). The main present
>> problem (which I don't see a new hypercall to solve) is that it's
>> GFN-based, and the GFN->MFN mapping can change after such
>> pinning got established. Otoh I think that by changing the
>> placement of the hvm_get_mem_pinned_cacheattr() calls we
>> could enforce the RAM-only aspect quite easily. Let me put
>> together a patch ...
> 
> That would be good. Thank you very much.

Actually here you go, albeit for now compile-tested only. Maybe
you, Andrew, or someone else has some early comment or
opinion on this already nevertheless. One thing to consider
cache flushing wise is whether when deleting a WC range it
wouldn't suffice to just force write combining buffers to be
cleared, instead of a full cache flush. But I guess that would
better be a 2nd patch anyway.

Jan

- call hvm_get_mem_pinned_cacheattr() for RAM ranges only (requires
  some re-ordering in epte_get_entry_emt(), to fully handle all MMIO
  aspects first)
- remove unnecessary indirection for hvm_get_mem_pinned_cacheattr()'s
  return of the type
- make hvm_set_mem_pinned_cacheattr() return an error on bad domain
  kind or obviously bad GFN range
- also avoid cache flush on EPT when removing a UC- range
- other code structure adjustments without intended functional change

--- unstable.orig/xen/arch/x86/hvm/mtrr.c
+++ unstable/xen/arch/x86/hvm/mtrr.c
@@ -521,14 +521,12 @@ struct hvm_mem_pinned_cacheattr_range {
 
 static DEFINE_RCU_READ_LOCK(pinned_cacheattr_rcu_lock);
 
-void hvm_init_cacheattr_region_list(
-    struct domain *d)
+void hvm_init_cacheattr_region_list(struct domain *d)
 {
     INIT_LIST_HEAD(&d->arch.hvm_domain.pinned_cacheattr_ranges);
 }
 
-void hvm_destroy_cacheattr_region_list(
-    struct domain *d)
+void hvm_destroy_cacheattr_region_list(struct domain *d)
 {
     struct list_head *head = &d->arch.hvm_domain.pinned_cacheattr_ranges;
     struct hvm_mem_pinned_cacheattr_range *range;
@@ -543,20 +541,14 @@ void hvm_destroy_cacheattr_region_list(
     }
 }
 
-int hvm_get_mem_pinned_cacheattr(
-    struct domain *d,
-    uint64_t guest_fn,
-    unsigned int order,
-    uint32_t *type)
+int hvm_get_mem_pinned_cacheattr(struct domain *d, uint64_t guest_fn,
+                                 unsigned int order)
 {
     struct hvm_mem_pinned_cacheattr_range *range;
     uint64_t mask = ~(uint64_t)0 << order;
-    int rc = 0;
+    int rc = -ENXIO;
 
-    *type = ~0;
-
-    if ( !is_hvm_domain(d) )
-        return 0;
+    ASSERT(has_hvm_container_domain(d));
 
     rcu_read_lock(&pinned_cacheattr_rcu_lock);
     list_for_each_entry_rcu ( range,
@@ -566,14 +558,13 @@ int hvm_get_mem_pinned_cacheattr(
         if ( ((guest_fn & mask) >= range->start) &&
              ((guest_fn | ~mask) <= range->end) )
         {
-            *type = range->type;
-            rc = 1;
+            rc = range->type;
             break;
         }
         if ( ((guest_fn & mask) <= range->end) &&
              (range->start <= (guest_fn | ~mask)) )
         {
-            rc = -1;
+            rc = -EADDRNOTAVAIL;
             break;
         }
     }
@@ -587,20 +578,21 @@ static void free_pinned_cacheattr_entry(
     xfree(container_of(rcu, struct hvm_mem_pinned_cacheattr_range, rcu));
 }
 
-int32_t hvm_set_mem_pinned_cacheattr(
-    struct domain *d,
-    uint64_t gfn_start,
-    uint64_t gfn_end,
-    uint32_t  type)
+int hvm_set_mem_pinned_cacheattr(struct domain *d, uint64_t gfn_start,
+                                 uint64_t gfn_end, uint32_t type)
 {
     struct hvm_mem_pinned_cacheattr_range *range;
     int rc = 1;
 
-    if ( !is_hvm_domain(d) || gfn_end < gfn_start )
-        return 0;
+    if ( !is_hvm_domain(d) )
+        return -EOPNOTSUPP;
+
+    if ( gfn_end < gfn_start || (gfn_start | gfn_end) >> paddr_bits )
+        return -EINVAL;
 
-    if ( type == XEN_DOMCTL_DELETE_MEM_CACHEATTR )
+    switch ( type )
     {
+    case XEN_DOMCTL_DELETE_MEM_CACHEATTR:
         /* Remove the requested range. */
         rcu_read_lock(&pinned_cacheattr_rcu_lock);
         list_for_each_entry_rcu ( range,
@@ -613,22 +605,37 @@ int32_t hvm_set_mem_pinned_cacheattr(
                 type = range->type;
                 call_rcu(&range->rcu, free_pinned_cacheattr_entry);
                 p2m_memory_type_changed(d);
-                if ( type != PAT_TYPE_UNCACHABLE )
+                switch ( type )
+                {
+                case PAT_TYPE_UC_MINUS:
+                    /*
+                     * For EPT we can also avoid the flush in this case;
+                     * see epte_get_entry_emt().
+                     */
+                    if ( hap_enabled(d) && cpu_has_vmx )
+                case PAT_TYPE_UNCACHABLE:
+                        break;
+                    /* fall through */
+                default:
                     flush_all(FLUSH_CACHE);
+                    break;
+                }
                 return 0;
             }
         rcu_read_unlock(&pinned_cacheattr_rcu_lock);
         return -ENOENT;
-    }
 
-    if ( !((type == PAT_TYPE_UNCACHABLE) ||
-           (type == PAT_TYPE_WRCOMB) ||
-           (type == PAT_TYPE_WRTHROUGH) ||
-           (type == PAT_TYPE_WRPROT) ||
-           (type == PAT_TYPE_WRBACK) ||
-           (type == PAT_TYPE_UC_MINUS)) ||
-         !is_hvm_domain(d) )
+    case PAT_TYPE_UC_MINUS:
+    case PAT_TYPE_UNCACHABLE:
+    case PAT_TYPE_WRBACK:
+    case PAT_TYPE_WRCOMB:
+    case PAT_TYPE_WRPROT:
+    case PAT_TYPE_WRTHROUGH:
+        break;
+
+    default:
         return -EINVAL;
+    }
 
     rcu_read_lock(&pinned_cacheattr_rcu_lock);
     list_for_each_entry_rcu ( range,
@@ -762,7 +769,6 @@ int epte_get_entry_emt(struct domain *d,
                        unsigned int order, uint8_t *ipat, bool_t direct_mmio)
 {
     int gmtrr_mtype, hmtrr_mtype;
-    uint32_t type;
     struct vcpu *v = current;
 
     *ipat = 0;
@@ -782,30 +788,28 @@ int epte_get_entry_emt(struct domain *d,
                                  mfn_x(mfn) + (1UL << order) - 1) )
         return -1;
 
-    switch ( hvm_get_mem_pinned_cacheattr(d, gfn, order, &type) )
+    if ( direct_mmio )
     {
-    case 1:
+        if ( (mfn_x(mfn) ^ d->arch.hvm_domain.vmx.apic_access_mfn) >> order )
+            return MTRR_TYPE_UNCACHABLE;
+        if ( order )
+            return -1;
         *ipat = 1;
-        return type != PAT_TYPE_UC_MINUS ? type : PAT_TYPE_UNCACHABLE;
-    case -1:
-        return -1;
+        return MTRR_TYPE_WRBACK;
     }
 
-    if ( !need_iommu(d) && !cache_flush_permitted(d) )
+    gmtrr_mtype = hvm_get_mem_pinned_cacheattr(d, gfn, order);
+    if ( gmtrr_mtype >= 0 )
     {
-        ASSERT(!direct_mmio ||
-               !((mfn_x(mfn) ^ d->arch.hvm_domain.vmx.apic_access_mfn) >>
-                 order));
         *ipat = 1;
-        return MTRR_TYPE_WRBACK;
+        return gmtrr_mtype != PAT_TYPE_UC_MINUS ? gmtrr_mtype
+                                                : MTRR_TYPE_UNCACHABLE;
     }
+    if ( gmtrr_mtype == -EADDRNOTAVAIL )
+        return -1;
 
-    if ( direct_mmio )
+    if ( !need_iommu(d) && !cache_flush_permitted(d) )
     {
-        if ( (mfn_x(mfn) ^ d->arch.hvm_domain.vmx.apic_access_mfn) >> order )
-            return MTRR_TYPE_UNCACHABLE;
-        if ( order )
-            return -1;
         *ipat = 1;
         return MTRR_TYPE_WRBACK;
     }
--- unstable.orig/xen/arch/x86/mm/shadow/multi.c
+++ unstable/xen/arch/x86/mm/shadow/multi.c
@@ -607,7 +607,7 @@ _sh_propagate(struct vcpu *v,
     if ( (level == 1) && is_hvm_domain(d) &&
          !is_xen_heap_mfn(mfn_x(target_mfn)) )
     {
-        unsigned int type;
+        int type;
 
         ASSERT(!(sflags & (_PAGE_PAT | _PAGE_PCD | _PAGE_PWT)));
 
@@ -618,7 +618,9 @@ _sh_propagate(struct vcpu *v,
          * 3) if disables snoop control, compute the PAT index with
          *    gMTRR and gPAT.
          */
-        if ( hvm_get_mem_pinned_cacheattr(d, gfn_x(target_gfn), 0, &type) )
+        if ( !mmio_mfn &&
+             (type = hvm_get_mem_pinned_cacheattr(d, gfn_x(target_gfn),
+                                                  0)) >= 0 )
             sflags |= pat_type_2_pte_flags(type);
         else if ( d->arch.hvm_domain.is_in_uc_mode )
             sflags |= pat_type_2_pte_flags(PAT_TYPE_UNCACHABLE);
--- unstable.orig/xen/include/asm-x86/hvm/cacheattr.h
+++ unstable/xen/include/asm-x86/hvm/cacheattr.h
@@ -1,29 +1,23 @@
 #ifndef __HVM_CACHEATTR_H__
 #define __HVM_CACHEATTR_H__
 
-void hvm_init_cacheattr_region_list(
-    struct domain *d);
-void hvm_destroy_cacheattr_region_list(
-    struct domain *d);
+#include <xen/types.h>
+
+struct domain;
+void hvm_init_cacheattr_region_list(struct domain *d);
+void hvm_destroy_cacheattr_region_list(struct domain *d);
 
 /*
  * To see guest_fn is in the pinned range or not,
- * if yes, return 1, and set type to value in this range
- * if no,  return 0, setting type to ~0
- * if ambiguous, return -1, setting type to ~0 (possible only for order > 0)
+ * if yes, return the (non-negative) type
+ * if no or ambiguous, return a negative error code
  */
-int hvm_get_mem_pinned_cacheattr(
-    struct domain *d,
-    uint64_t guest_fn,
-    unsigned int order,
-    uint32_t *type);
+int hvm_get_mem_pinned_cacheattr(struct domain *d, uint64_t guest_fn,
+                                 unsigned int order);
 
 
 /* Set pinned caching type for a domain. */
-int32_t hvm_set_mem_pinned_cacheattr(
-    struct domain *d,
-    uint64_t gfn_start,
-    uint64_t gfn_end,
-    uint32_t  type);
+int hvm_set_mem_pinned_cacheattr(struct domain *d, uint64_t gfn_start,
+                                 uint64_t gfn_end, uint32_t type);
 
 #endif /* __HVM_CACHEATTR_H__ */


[-- Attachment #2: x86-HVM-mem-pinned-cacheattr.patch --]
[-- Type: text/plain, Size: 9143 bytes --]


- call hvm_get_mem_pinned_cacheattr() for RAM ranges only (requires
  some re-ordering in epte_get_entry_emt(), to fully handle all MMIO
  aspects first)
- remove unnecessary indirection for hvm_get_mem_pinned_cacheattr()'s
  return of the type
- make hvm_set_mem_pinned_cacheattr() return an error on bad domain
  kind or obviously bad GFN range
- also avoid cache flush on EPT when removing a UC- range
- other code structure adjustments without intended functional change

--- unstable.orig/xen/arch/x86/hvm/mtrr.c
+++ unstable/xen/arch/x86/hvm/mtrr.c
@@ -521,14 +521,12 @@ struct hvm_mem_pinned_cacheattr_range {
 
 static DEFINE_RCU_READ_LOCK(pinned_cacheattr_rcu_lock);
 
-void hvm_init_cacheattr_region_list(
-    struct domain *d)
+void hvm_init_cacheattr_region_list(struct domain *d)
 {
     INIT_LIST_HEAD(&d->arch.hvm_domain.pinned_cacheattr_ranges);
 }
 
-void hvm_destroy_cacheattr_region_list(
-    struct domain *d)
+void hvm_destroy_cacheattr_region_list(struct domain *d)
 {
     struct list_head *head = &d->arch.hvm_domain.pinned_cacheattr_ranges;
     struct hvm_mem_pinned_cacheattr_range *range;
@@ -543,20 +541,14 @@ void hvm_destroy_cacheattr_region_list(
     }
 }
 
-int hvm_get_mem_pinned_cacheattr(
-    struct domain *d,
-    uint64_t guest_fn,
-    unsigned int order,
-    uint32_t *type)
+int hvm_get_mem_pinned_cacheattr(struct domain *d, uint64_t guest_fn,
+                                 unsigned int order)
 {
     struct hvm_mem_pinned_cacheattr_range *range;
     uint64_t mask = ~(uint64_t)0 << order;
-    int rc = 0;
+    int rc = -ENXIO;
 
-    *type = ~0;
-
-    if ( !is_hvm_domain(d) )
-        return 0;
+    ASSERT(has_hvm_container_domain(d));
 
     rcu_read_lock(&pinned_cacheattr_rcu_lock);
     list_for_each_entry_rcu ( range,
@@ -566,14 +558,13 @@ int hvm_get_mem_pinned_cacheattr(
         if ( ((guest_fn & mask) >= range->start) &&
              ((guest_fn | ~mask) <= range->end) )
         {
-            *type = range->type;
-            rc = 1;
+            rc = range->type;
             break;
         }
         if ( ((guest_fn & mask) <= range->end) &&
              (range->start <= (guest_fn | ~mask)) )
         {
-            rc = -1;
+            rc = -EADDRNOTAVAIL;
             break;
         }
     }
@@ -587,20 +578,21 @@ static void free_pinned_cacheattr_entry(
     xfree(container_of(rcu, struct hvm_mem_pinned_cacheattr_range, rcu));
 }
 
-int32_t hvm_set_mem_pinned_cacheattr(
-    struct domain *d,
-    uint64_t gfn_start,
-    uint64_t gfn_end,
-    uint32_t  type)
+int hvm_set_mem_pinned_cacheattr(struct domain *d, uint64_t gfn_start,
+                                 uint64_t gfn_end, uint32_t type)
 {
     struct hvm_mem_pinned_cacheattr_range *range;
     int rc = 1;
 
-    if ( !is_hvm_domain(d) || gfn_end < gfn_start )
-        return 0;
+    if ( !is_hvm_domain(d) )
+        return -EOPNOTSUPP;
+
+    if ( gfn_end < gfn_start || (gfn_start | gfn_end) >> paddr_bits )
+        return -EINVAL;
 
-    if ( type == XEN_DOMCTL_DELETE_MEM_CACHEATTR )
+    switch ( type )
     {
+    case XEN_DOMCTL_DELETE_MEM_CACHEATTR:
         /* Remove the requested range. */
         rcu_read_lock(&pinned_cacheattr_rcu_lock);
         list_for_each_entry_rcu ( range,
@@ -613,22 +605,37 @@ int32_t hvm_set_mem_pinned_cacheattr(
                 type = range->type;
                 call_rcu(&range->rcu, free_pinned_cacheattr_entry);
                 p2m_memory_type_changed(d);
-                if ( type != PAT_TYPE_UNCACHABLE )
+                switch ( type )
+                {
+                case PAT_TYPE_UC_MINUS:
+                    /*
+                     * For EPT we can also avoid the flush in this case;
+                     * see epte_get_entry_emt().
+                     */
+                    if ( hap_enabled(d) && cpu_has_vmx )
+                case PAT_TYPE_UNCACHABLE:
+                        break;
+                    /* fall through */
+                default:
                     flush_all(FLUSH_CACHE);
+                    break;
+                }
                 return 0;
             }
         rcu_read_unlock(&pinned_cacheattr_rcu_lock);
         return -ENOENT;
-    }
 
-    if ( !((type == PAT_TYPE_UNCACHABLE) ||
-           (type == PAT_TYPE_WRCOMB) ||
-           (type == PAT_TYPE_WRTHROUGH) ||
-           (type == PAT_TYPE_WRPROT) ||
-           (type == PAT_TYPE_WRBACK) ||
-           (type == PAT_TYPE_UC_MINUS)) ||
-         !is_hvm_domain(d) )
+    case PAT_TYPE_UC_MINUS:
+    case PAT_TYPE_UNCACHABLE:
+    case PAT_TYPE_WRBACK:
+    case PAT_TYPE_WRCOMB:
+    case PAT_TYPE_WRPROT:
+    case PAT_TYPE_WRTHROUGH:
+        break;
+
+    default:
         return -EINVAL;
+    }
 
     rcu_read_lock(&pinned_cacheattr_rcu_lock);
     list_for_each_entry_rcu ( range,
@@ -762,7 +769,6 @@ int epte_get_entry_emt(struct domain *d,
                        unsigned int order, uint8_t *ipat, bool_t direct_mmio)
 {
     int gmtrr_mtype, hmtrr_mtype;
-    uint32_t type;
     struct vcpu *v = current;
 
     *ipat = 0;
@@ -782,30 +788,28 @@ int epte_get_entry_emt(struct domain *d,
                                  mfn_x(mfn) + (1UL << order) - 1) )
         return -1;
 
-    switch ( hvm_get_mem_pinned_cacheattr(d, gfn, order, &type) )
+    if ( direct_mmio )
     {
-    case 1:
+        if ( (mfn_x(mfn) ^ d->arch.hvm_domain.vmx.apic_access_mfn) >> order )
+            return MTRR_TYPE_UNCACHABLE;
+        if ( order )
+            return -1;
         *ipat = 1;
-        return type != PAT_TYPE_UC_MINUS ? type : PAT_TYPE_UNCACHABLE;
-    case -1:
-        return -1;
+        return MTRR_TYPE_WRBACK;
     }
 
-    if ( !need_iommu(d) && !cache_flush_permitted(d) )
+    gmtrr_mtype = hvm_get_mem_pinned_cacheattr(d, gfn, order);
+    if ( gmtrr_mtype >= 0 )
     {
-        ASSERT(!direct_mmio ||
-               !((mfn_x(mfn) ^ d->arch.hvm_domain.vmx.apic_access_mfn) >>
-                 order));
         *ipat = 1;
-        return MTRR_TYPE_WRBACK;
+        return gmtrr_mtype != PAT_TYPE_UC_MINUS ? gmtrr_mtype
+                                                : MTRR_TYPE_UNCACHABLE;
     }
+    if ( gmtrr_mtype == -EADDRNOTAVAIL )
+        return -1;
 
-    if ( direct_mmio )
+    if ( !need_iommu(d) && !cache_flush_permitted(d) )
     {
-        if ( (mfn_x(mfn) ^ d->arch.hvm_domain.vmx.apic_access_mfn) >> order )
-            return MTRR_TYPE_UNCACHABLE;
-        if ( order )
-            return -1;
         *ipat = 1;
         return MTRR_TYPE_WRBACK;
     }
--- unstable.orig/xen/arch/x86/mm/shadow/multi.c
+++ unstable/xen/arch/x86/mm/shadow/multi.c
@@ -607,7 +607,7 @@ _sh_propagate(struct vcpu *v,
     if ( (level == 1) && is_hvm_domain(d) &&
          !is_xen_heap_mfn(mfn_x(target_mfn)) )
     {
-        unsigned int type;
+        int type;
 
         ASSERT(!(sflags & (_PAGE_PAT | _PAGE_PCD | _PAGE_PWT)));
 
@@ -618,7 +618,9 @@ _sh_propagate(struct vcpu *v,
          * 3) if disables snoop control, compute the PAT index with
          *    gMTRR and gPAT.
          */
-        if ( hvm_get_mem_pinned_cacheattr(d, gfn_x(target_gfn), 0, &type) )
+        if ( !mmio_mfn &&
+             (type = hvm_get_mem_pinned_cacheattr(d, gfn_x(target_gfn),
+                                                  0)) >= 0 )
             sflags |= pat_type_2_pte_flags(type);
         else if ( d->arch.hvm_domain.is_in_uc_mode )
             sflags |= pat_type_2_pte_flags(PAT_TYPE_UNCACHABLE);
--- unstable.orig/xen/include/asm-x86/hvm/cacheattr.h
+++ unstable/xen/include/asm-x86/hvm/cacheattr.h
@@ -1,29 +1,23 @@
 #ifndef __HVM_CACHEATTR_H__
 #define __HVM_CACHEATTR_H__
 
-void hvm_init_cacheattr_region_list(
-    struct domain *d);
-void hvm_destroy_cacheattr_region_list(
-    struct domain *d);
+#include <xen/types.h>
+
+struct domain;
+void hvm_init_cacheattr_region_list(struct domain *d);
+void hvm_destroy_cacheattr_region_list(struct domain *d);
 
 /*
  * To see guest_fn is in the pinned range or not,
- * if yes, return 1, and set type to value in this range
- * if no,  return 0, setting type to ~0
- * if ambiguous, return -1, setting type to ~0 (possible only for order > 0)
+ * if yes, return the (non-negative) type
+ * if no or ambiguous, return a negative error code
  */
-int hvm_get_mem_pinned_cacheattr(
-    struct domain *d,
-    uint64_t guest_fn,
-    unsigned int order,
-    uint32_t *type);
+int hvm_get_mem_pinned_cacheattr(struct domain *d, uint64_t guest_fn,
+                                 unsigned int order);
 
 
 /* Set pinned caching type for a domain. */
-int32_t hvm_set_mem_pinned_cacheattr(
-    struct domain *d,
-    uint64_t gfn_start,
-    uint64_t gfn_end,
-    uint32_t  type);
+int hvm_set_mem_pinned_cacheattr(struct domain *d, uint64_t gfn_start,
+                                 uint64_t gfn_end, uint32_t type);
 
 #endif /* __HVM_CACHEATTR_H__ */

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel