Re: [Xen-devel] [PATCH 5/5] docs/pvh: document initial MTRR state

[Jan Beulich]

>>> On 15.05.18 at 10:30,  wrote:
> On Tue, May 15, 2018 at 01:51:03AM -0600, Jan Beulich wrote:
>> >>> On 14.05.18 at 18:18,  wrote:
>> > On Mon, May 14, 2018 at 10:13:47AM -0600, Jan Beulich wrote:
>> >> >>> On 14.05.18 at 18:03,  wrote:
>> >> > On Thu, May 10, 2018 at 06:15:05PM +0100, Roger Pau Monne wrote:
>> >> >> --- a/docs/misc/pvh.markdown
>> >> >> +++ b/docs/misc/pvh.markdown
>> >> >> @@ -92,3 +92,18 @@ event channels. Delivery of those interrupts can be 
> configured in the same way
>> >> >>  as HVM guests, check xen/include/public/hvm/params.h and
>> >> >>  xen/include/public/hvm/hvm\_op.h for more information about available 
> delivery
>> >> >>  methods.
>> >> >> +
>> >> >> +## MTRR ##
>> >> >> +
>> >> >> +### Unprivileged guests ###
>> >> >> +
>> >> >> +PVH guests are booted with the default MTRR type set to write-back 
>> >> >> and 
> MTRR
>> >> >> +enabled. This allows DomUs to start with a sane MTRR state. Note that 
> this will
>> >> >> +have to be revisited when pci-passthrough is added to PVH in order to 
>> >> >> set 
> MMIO
>> >> >> +regions as UC.
>> >> > 
>> >> > My reading is "revisited" implies the default type will change. In fact
>> >> > it shouldn't. We should clarify: for ram it will remain WB, for MMIO
>> >> > holes it will be UC.
>> >> 
>> >> Why would changing the default late be a problem? A firmware update on
>> >> bare hardware might also have such an effect. The default type read from
>> >> the MSR must not change across the lifetime of a VM, but imo may change
>> >> across reboots of it.
>> >> 
>> > 
>> > Then setting a default here doesn't really help OS developers because
>> > they will always need to write code to set the correct type -- not that
>> > this is a big issue, but as I understand it the point here is to avoid
>> > that.
>> 
>> Hmm, my understanding of the purpose of the series was that it aims at
>> establishing some sane (i.e. reasonable for an OS to expect) state, instead
>> of a firm "this will always be this way" one. Furthermore OSes generally
>> shouldn't find a need to fiddle with MTRRs, provided firmware has done a
>> proper job setting them up.
> 
> Indeed that's the purpose. Most OSes don't really care about the
> details of the MTRR setup, and they just expect RAM regions to be set
> to WB and MMIO holes to UC AFAICT.
> 
> I don't think Xen has to provide any guarantee about the details of
> the MTRR state, apart from stating that RAM will be WB and MMIO UC.
> 
> I can leave the text as-is, or add the paragraph suggested in another
> email to clarify if the current writing is prone to misunderstanding.

I indeed think the text as still visible above is not sufficiently clear (as in:
is not leaving sufficient leeway for future adjustments), so I'd prefer if
the clarification from the other sub-thread was used (as replacement or
addition).

Jan



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 5/5] docs/pvh: document initial MTRR state

[Roger Pau Monné]

On Tue, May 15, 2018 at 01:51:03AM -0600, Jan Beulich wrote:
> >>> On 14.05.18 at 18:18,  wrote:
> > On Mon, May 14, 2018 at 10:13:47AM -0600, Jan Beulich wrote:
> >> >>> On 14.05.18 at 18:03,  wrote:
> >> > On Thu, May 10, 2018 at 06:15:05PM +0100, Roger Pau Monne wrote:
> >> >> --- a/docs/misc/pvh.markdown
> >> >> +++ b/docs/misc/pvh.markdown
> >> >> @@ -92,3 +92,18 @@ event channels. Delivery of those interrupts can be 
> >> >> configured in the same way
> >> >>  as HVM guests, check xen/include/public/hvm/params.h and
> >> >>  xen/include/public/hvm/hvm\_op.h for more information about available 
> >> >> delivery
> >> >>  methods.
> >> >> +
> >> >> +## MTRR ##
> >> >> +
> >> >> +### Unprivileged guests ###
> >> >> +
> >> >> +PVH guests are booted with the default MTRR type set to write-back and 
> >> >> MTRR
> >> >> +enabled. This allows DomUs to start with a sane MTRR state. Note that 
> >> >> this will
> >> >> +have to be revisited when pci-passthrough is added to PVH in order to 
> >> >> set MMIO
> >> >> +regions as UC.
> >> > 
> >> > My reading is "revisited" implies the default type will change. In fact
> >> > it shouldn't. We should clarify: for ram it will remain WB, for MMIO
> >> > holes it will be UC.
> >> 
> >> Why would changing the default late be a problem? A firmware update on
> >> bare hardware might also have such an effect. The default type read from
> >> the MSR must not change across the lifetime of a VM, but imo may change
> >> across reboots of it.
> >> 
> > 
> > Then setting a default here doesn't really help OS developers because
> > they will always need to write code to set the correct type -- not that
> > this is a big issue, but as I understand it the point here is to avoid
> > that.
> 
> Hmm, my understanding of the purpose of the series was that it aims at
> establishing some sane (i.e. reasonable for an OS to expect) state, instead
> of a firm "this will always be this way" one. Furthermore OSes generally
> shouldn't find a need to fiddle with MTRRs, provided firmware has done a
> proper job setting them up.

Indeed that's the purpose. Most OSes don't really care about the
details of the MTRR setup, and they just expect RAM regions to be set
to WB and MMIO holes to UC AFAICT.

I don't think Xen has to provide any guarantee about the details of
the MTRR state, apart from stating that RAM will be WB and MMIO UC.

I can leave the text as-is, or add the paragraph suggested in another
email to clarify if the current writing is prone to misunderstanding.

Roger.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 5/5] docs/pvh: document initial MTRR state

[Wei Liu]

On Tue, May 15, 2018 at 01:51:03AM -0600, Jan Beulich wrote:
> >>> On 14.05.18 at 18:18,  wrote:
> > On Mon, May 14, 2018 at 10:13:47AM -0600, Jan Beulich wrote:
> >> >>> On 14.05.18 at 18:03,  wrote:
> >> > On Thu, May 10, 2018 at 06:15:05PM +0100, Roger Pau Monne wrote:
> >> >> --- a/docs/misc/pvh.markdown
> >> >> +++ b/docs/misc/pvh.markdown
> >> >> @@ -92,3 +92,18 @@ event channels. Delivery of those interrupts can be 
> >> >> configured in the same way
> >> >>  as HVM guests, check xen/include/public/hvm/params.h and
> >> >>  xen/include/public/hvm/hvm\_op.h for more information about available 
> >> >> delivery
> >> >>  methods.
> >> >> +
> >> >> +## MTRR ##
> >> >> +
> >> >> +### Unprivileged guests ###
> >> >> +
> >> >> +PVH guests are booted with the default MTRR type set to write-back and 
> >> >> MTRR
> >> >> +enabled. This allows DomUs to start with a sane MTRR state. Note that 
> >> >> this will
> >> >> +have to be revisited when pci-passthrough is added to PVH in order to 
> >> >> set MMIO
> >> >> +regions as UC.
> >> > 
> >> > My reading is "revisited" implies the default type will change. In fact
> >> > it shouldn't. We should clarify: for ram it will remain WB, for MMIO
> >> > holes it will be UC.
> >> 
> >> Why would changing the default late be a problem? A firmware update on
> >> bare hardware might also have such an effect. The default type read from
> >> the MSR must not change across the lifetime of a VM, but imo may change
> >> across reboots of it.
> >> 
> > 
> > Then setting a default here doesn't really help OS developers because
> > they will always need to write code to set the correct type -- not that
> > this is a big issue, but as I understand it the point here is to avoid
> > that.
> 
> Hmm, my understanding of the purpose of the series was that it aims at
> establishing some sane (i.e. reasonable for an OS to expect) state, instead
> of a firm "this will always be this way" one. Furthermore OSes generally
> shouldn't find a need to fiddle with MTRRs, provided firmware has done a
> proper job setting them up.

AIUI this series is the result of discussion of  [PATCH v2 1/3] xen/pvh:
enable and set default MTRR type.

It appears because the default is not sane, other pieces of software
(hvmloader, ovmf, now PVH kernel) have to set it again and again and
again.

Fundamentally I don't think we disagree with each other. If we go with
Roger's suggestion in the other sub-thread, we can ensure sane types for
RAM and MMIO and avoid debating here at all.

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 5/5] docs/pvh: document initial MTRR state

[Jan Beulich]

>>> On 14.05.18 at 18:18,  wrote:
> On Mon, May 14, 2018 at 10:13:47AM -0600, Jan Beulich wrote:
>> >>> On 14.05.18 at 18:03,  wrote:
>> > On Thu, May 10, 2018 at 06:15:05PM +0100, Roger Pau Monne wrote:
>> >> --- a/docs/misc/pvh.markdown
>> >> +++ b/docs/misc/pvh.markdown
>> >> @@ -92,3 +92,18 @@ event channels. Delivery of those interrupts can be 
>> >> configured in the same way
>> >>  as HVM guests, check xen/include/public/hvm/params.h and
>> >>  xen/include/public/hvm/hvm\_op.h for more information about available 
>> >> delivery
>> >>  methods.
>> >> +
>> >> +## MTRR ##
>> >> +
>> >> +### Unprivileged guests ###
>> >> +
>> >> +PVH guests are booted with the default MTRR type set to write-back and 
>> >> MTRR
>> >> +enabled. This allows DomUs to start with a sane MTRR state. Note that 
>> >> this will
>> >> +have to be revisited when pci-passthrough is added to PVH in order to 
>> >> set MMIO
>> >> +regions as UC.
>> > 
>> > My reading is "revisited" implies the default type will change. In fact
>> > it shouldn't. We should clarify: for ram it will remain WB, for MMIO
>> > holes it will be UC.
>> 
>> Why would changing the default late be a problem? A firmware update on
>> bare hardware might also have such an effect. The default type read from
>> the MSR must not change across the lifetime of a VM, but imo may change
>> across reboots of it.
>> 
> 
> Then setting a default here doesn't really help OS developers because
> they will always need to write code to set the correct type -- not that
> this is a big issue, but as I understand it the point here is to avoid
> that.

Hmm, my understanding of the purpose of the series was that it aims at
establishing some sane (i.e. reasonable for an OS to expect) state, instead
of a firm "this will always be this way" one. Furthermore OSes generally
shouldn't find a need to fiddle with MTRRs, provided firmware has done a
proper job setting them up.

Jan



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 5/5] docs/pvh: document initial MTRR state

[Wei Liu]

On Mon, May 14, 2018 at 05:16:10PM +0100, Roger Pau Monné wrote:
> On Mon, May 14, 2018 at 05:03:52PM +0100, Wei Liu wrote:
> > On Thu, May 10, 2018 at 06:15:05PM +0100, Roger Pau Monne wrote:
> > > Provided to both Dom0 and DomUs.
> > > 
> > > Signed-off-by: Roger Pau Monné 
> > > ---
> > > Cc: Andrew Cooper 
> > > Cc: George Dunlap 
> > > Cc: Ian Jackson 
> > > Cc: Jan Beulich 
> > > Cc: Julien Grall 
> > > Cc: Konrad Rzeszutek Wilk 
> > > Cc: Stefano Stabellini 
> > > Cc: Tim Deegan 
> > > Cc: Wei Liu 
> > > ---
> > >  docs/misc/pvh.markdown | 15 +++
> > >  1 file changed, 15 insertions(+)
> > > 
> > > diff --git a/docs/misc/pvh.markdown b/docs/misc/pvh.markdown
> > > index e85fb15374..639401a887 100644
> > > --- a/docs/misc/pvh.markdown
> > > +++ b/docs/misc/pvh.markdown
> > > @@ -92,3 +92,18 @@ event channels. Delivery of those interrupts can be 
> > > configured in the same way
> > >  as HVM guests, check xen/include/public/hvm/params.h and
> > >  xen/include/public/hvm/hvm\_op.h for more information about available 
> > > delivery
> > >  methods.
> > > +
> > > +## MTRR ##
> > > +
> > > +### Unprivileged guests ###
> > > +
> > > +PVH guests are booted with the default MTRR type set to write-back and 
> > > MTRR
> > > +enabled. This allows DomUs to start with a sane MTRR state. Note that 
> > > this will
> > > +have to be revisited when pci-passthrough is added to PVH in order to 
> > > set MMIO
> > > +regions as UC.
> > 
> > My reading is "revisited" implies the default type will change. In fact
> > it shouldn't. We should clarify: for ram it will remain WB, for MMIO
> > holes it will be UC.
> >
> > Please correct me if I'm wrong.
> 
> That's correct. I've used "revisited" here in the sense that Xen might
> change the default type to UC and set the RAM regions as WB using
> variable MTRR ranges for example.
> 
> I simply wanted to remark that the way RAM is set to WB is currently
> done using the default MTRR type. RAM will always be set of WB for PVH
> in MTRR, however the way to achieve it might change.
> 
> What about adding:
> 
> "Xen guarantees that RAM regions will always have the WB cache type
> set in the initial MTRR state, either set by the default MTRR type or
> by other means."
> 

Sounds good to me.

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 5/5] docs/pvh: document initial MTRR state

[Wei Liu]

On Mon, May 14, 2018 at 10:13:47AM -0600, Jan Beulich wrote:
> >>> On 14.05.18 at 18:03,  wrote:
> > On Thu, May 10, 2018 at 06:15:05PM +0100, Roger Pau Monne wrote:
> >> --- a/docs/misc/pvh.markdown
> >> +++ b/docs/misc/pvh.markdown
> >> @@ -92,3 +92,18 @@ event channels. Delivery of those interrupts can be 
> >> configured in the same way
> >>  as HVM guests, check xen/include/public/hvm/params.h and
> >>  xen/include/public/hvm/hvm\_op.h for more information about available 
> >> delivery
> >>  methods.
> >> +
> >> +## MTRR ##
> >> +
> >> +### Unprivileged guests ###
> >> +
> >> +PVH guests are booted with the default MTRR type set to write-back and 
> >> MTRR
> >> +enabled. This allows DomUs to start with a sane MTRR state. Note that 
> >> this will
> >> +have to be revisited when pci-passthrough is added to PVH in order to set 
> >> MMIO
> >> +regions as UC.
> > 
> > My reading is "revisited" implies the default type will change. In fact
> > it shouldn't. We should clarify: for ram it will remain WB, for MMIO
> > holes it will be UC.
> 
> Why would changing the default late be a problem? A firmware update on
> bare hardware might also have such an effect. The default type read from
> the MSR must not change across the lifetime of a VM, but imo may change
> across reboots of it.
> 

Then setting a default here doesn't really help OS developers because
they will always need to write code to set the correct type -- not that
this is a big issue, but as I understand it the point here is to avoid
that.

Wei.

> Jan
> 
> 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 5/5] docs/pvh: document initial MTRR state

[Roger Pau Monné]

On Mon, May 14, 2018 at 05:03:52PM +0100, Wei Liu wrote:
> On Thu, May 10, 2018 at 06:15:05PM +0100, Roger Pau Monne wrote:
> > Provided to both Dom0 and DomUs.
> > 
> > Signed-off-by: Roger Pau Monné 
> > ---
> > Cc: Andrew Cooper 
> > Cc: George Dunlap 
> > Cc: Ian Jackson 
> > Cc: Jan Beulich 
> > Cc: Julien Grall 
> > Cc: Konrad Rzeszutek Wilk 
> > Cc: Stefano Stabellini 
> > Cc: Tim Deegan 
> > Cc: Wei Liu 
> > ---
> >  docs/misc/pvh.markdown | 15 +++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/docs/misc/pvh.markdown b/docs/misc/pvh.markdown
> > index e85fb15374..639401a887 100644
> > --- a/docs/misc/pvh.markdown
> > +++ b/docs/misc/pvh.markdown
> > @@ -92,3 +92,18 @@ event channels. Delivery of those interrupts can be 
> > configured in the same way
> >  as HVM guests, check xen/include/public/hvm/params.h and
> >  xen/include/public/hvm/hvm\_op.h for more information about available 
> > delivery
> >  methods.
> > +
> > +## MTRR ##
> > +
> > +### Unprivileged guests ###
> > +
> > +PVH guests are booted with the default MTRR type set to write-back and MTRR
> > +enabled. This allows DomUs to start with a sane MTRR state. Note that this 
> > will
> > +have to be revisited when pci-passthrough is added to PVH in order to set 
> > MMIO
> > +regions as UC.
> 
> My reading is "revisited" implies the default type will change. In fact
> it shouldn't. We should clarify: for ram it will remain WB, for MMIO
> holes it will be UC.
>
> Please correct me if I'm wrong.

That's correct. I've used "revisited" here in the sense that Xen might
change the default type to UC and set the RAM regions as WB using
variable MTRR ranges for example.

I simply wanted to remark that the way RAM is set to WB is currently
done using the default MTRR type. RAM will always be set of WB for PVH
in MTRR, however the way to achieve it might change.

What about adding:

"Xen guarantees that RAM regions will always have the WB cache type
set in the initial MTRR state, either set by the default MTRR type or
by other means."

Thanks, Roger.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 5/5] docs/pvh: document initial MTRR state

[Jan Beulich]

>>> On 14.05.18 at 18:03,  wrote:
> On Thu, May 10, 2018 at 06:15:05PM +0100, Roger Pau Monne wrote:
>> --- a/docs/misc/pvh.markdown
>> +++ b/docs/misc/pvh.markdown
>> @@ -92,3 +92,18 @@ event channels. Delivery of those interrupts can be 
>> configured in the same way
>>  as HVM guests, check xen/include/public/hvm/params.h and
>>  xen/include/public/hvm/hvm\_op.h for more information about available 
>> delivery
>>  methods.
>> +
>> +## MTRR ##
>> +
>> +### Unprivileged guests ###
>> +
>> +PVH guests are booted with the default MTRR type set to write-back and MTRR
>> +enabled. This allows DomUs to start with a sane MTRR state. Note that this 
>> will
>> +have to be revisited when pci-passthrough is added to PVH in order to set 
>> MMIO
>> +regions as UC.
> 
> My reading is "revisited" implies the default type will change. In fact
> it shouldn't. We should clarify: for ram it will remain WB, for MMIO
> holes it will be UC.

Why would changing the default late be a problem? A firmware update on
bare hardware might also have such an effect. The default type read from
the MSR must not change across the lifetime of a VM, but imo may change
across reboots of it.

Jan



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 5/5] docs/pvh: document initial MTRR state

[Wei Liu]

On Thu, May 10, 2018 at 06:15:05PM +0100, Roger Pau Monne wrote:
> Provided to both Dom0 and DomUs.
> 
> Signed-off-by: Roger Pau Monné 
> ---
> Cc: Andrew Cooper 
> Cc: George Dunlap 
> Cc: Ian Jackson 
> Cc: Jan Beulich 
> Cc: Julien Grall 
> Cc: Konrad Rzeszutek Wilk 
> Cc: Stefano Stabellini 
> Cc: Tim Deegan 
> Cc: Wei Liu 
> ---
>  docs/misc/pvh.markdown | 15 +++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/docs/misc/pvh.markdown b/docs/misc/pvh.markdown
> index e85fb15374..639401a887 100644
> --- a/docs/misc/pvh.markdown
> +++ b/docs/misc/pvh.markdown
> @@ -92,3 +92,18 @@ event channels. Delivery of those interrupts can be 
> configured in the same way
>  as HVM guests, check xen/include/public/hvm/params.h and
>  xen/include/public/hvm/hvm\_op.h for more information about available 
> delivery
>  methods.
> +
> +## MTRR ##
> +
> +### Unprivileged guests ###
> +
> +PVH guests are booted with the default MTRR type set to write-back and MTRR
> +enabled. This allows DomUs to start with a sane MTRR state. Note that this 
> will
> +have to be revisited when pci-passthrough is added to PVH in order to set 
> MMIO
> +regions as UC.

My reading is "revisited" implies the default type will change. In fact
it shouldn't. We should clarify: for ram it will remain WB, for MMIO
holes it will be UC.

Please correct me if I'm wrong.

Wei.

> +
> +### Hardware domain ###
> +
> +A PVH hardware domain is booted with the same MTRR state as the one found on
> +the host. This is done because the hardware domain memory map is already a
> +modified copy of the host memory map, so the same MTRR setup should work.
> -- 
> 2.17.0
> 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 5/5] docs/pvh: document initial MTRR state

[Roger Pau Monne]

Provided to both Dom0 and DomUs.

Signed-off-by: Roger Pau Monné 
---
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Julien Grall 
Cc: Konrad Rzeszutek Wilk 
Cc: Stefano Stabellini 
Cc: Tim Deegan 
Cc: Wei Liu 
---
 docs/misc/pvh.markdown | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/docs/misc/pvh.markdown b/docs/misc/pvh.markdown
index e85fb15374..639401a887 100644
--- a/docs/misc/pvh.markdown
+++ b/docs/misc/pvh.markdown
@@ -92,3 +92,18 @@ event channels. Delivery of those interrupts can be 
configured in the same way
 as HVM guests, check xen/include/public/hvm/params.h and
 xen/include/public/hvm/hvm\_op.h for more information about available delivery
 methods.
+
+## MTRR ##
+
+### Unprivileged guests ###
+
+PVH guests are booted with the default MTRR type set to write-back and MTRR
+enabled. This allows DomUs to start with a sane MTRR state. Note that this will
+have to be revisited when pci-passthrough is added to PVH in order to set MMIO
+regions as UC.
+
+### Hardware domain ###
+
+A PVH hardware domain is booted with the same MTRR state as the one found on
+the host. This is done because the hardware domain memory map is already a
+modified copy of the host memory map, so the same MTRR setup should work.
-- 
2.17.0


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel