[Xen-devel] Backports to 4.13

[Xen-devel] Backports to 4.13

[Andrew Cooper]

Hello,

You've recently backported 328dd238da "x86/sm{e, a}p: do not enable
SMEP/SMAP in PV shim by default on AMD", but have missed 5de961d9c097
"x86: do not enable global pages when virtualized on AMD or Hygon
hardware" and b041709c369b "x86/pv: Fix `global-pages` to match the
documentation".

The former is pretty useless without the other two, because you're
taking out the security feature without gaining the majority performance
win from avoiding VMexits due to CR4 traps.

Depending on the view of other PV shim usability issues, 6dd95b02ea27
and f9dee1f945eb regarding vtsc make a large difference and should be
considered.

e2d1254f5af2 is a bugfix which needs backporting to fix AMD S3 and
kexec, as well as pv shim.

188f479de4b7 and 005de45c887e are both core scheduling bugfixes and
should be considered, even if they are a little too fresh right at the
moment.


On the tools side of things, f50a4f6e244c aafae0e800e9 2a62c22715bf
d79cc6bc2bac 0729830cc425 are all bugs in CPUID and/or migration.  "Fix
HVM_PARAM_PAE_ENABLED handling" is only for 4.13, whereas all the others
are back to 4.7 (technically speaking).

~Andrew

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

Re: [Xen-devel] Backports to 4.13

[Jan Beulich]

On 06.03.2020 14:52, Andrew Cooper wrote:
> You've recently backported 328dd238da "x86/sm{e, a}p: do not enable
> SMEP/SMAP in PV shim by default on AMD", but have missed 5de961d9c097
> "x86: do not enable global pages when virtualized on AMD or Hygon
> hardware" and b041709c369b "x86/pv: Fix `global-pages` to match the
> documentation".

I've queued these up, albeit ...

> The former is pretty useless without the other two, because you're
> taking out the security feature without gaining the majority performance
> win from avoiding VMexits due to CR4 traps.

... I'm unconvinced of this - avoiding SMEP/SMAP alone already
eliminates a number of CR4 writes afaict.

> Depending on the view of other PV shim usability issues, 6dd95b02ea27
> and f9dee1f945eb regarding vtsc make a large difference and should be
> considered.

I've queued the latter, but the former doesn't really look like
backporting material to me.

> e2d1254f5af2 is a bugfix which needs backporting to fix AMD S3 and
> kexec, as well as pv shim.

Queued as well.

> 188f479de4b7 and 005de45c887e are both core scheduling bugfixes and
> should be considered, even if they are a little too fresh right at the
> moment.

"Freshness" is not an issue imo. They've passed the push gate on
master, and I wouldn't get around to apply them right away anyway.
With these it's just the typical situation for features that are
still "experimental" or alike - I'm never really certain whether
it's better to pull in such fixes (and have more code churn) or
to leave them out. And explicit request like this one of yours
helps take a decision.

> On the tools side of things, f50a4f6e244c aafae0e800e9 2a62c22715bf
> d79cc6bc2bac 0729830cc425 are all bugs in CPUID and/or migration.  "Fix
> HVM_PARAM_PAE_ENABLED handling" is only for 4.13, whereas all the others
> are back to 4.7 (technically speaking).

I'll (obviously) leave these to Ian.

Jan

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

Re: [Xen-devel] Backports to 4.13

[Andrew Cooper]

On 06/03/2020 14:11, Jan Beulich wrote:
> On 06.03.2020 14:52, Andrew Cooper wrote:
>> The former is pretty useless without the other two, because you're
>> taking out the security feature without gaining the majority performance
>> win from avoiding VMexits due to CR4 traps.
> ... I'm unconvinced of this - avoiding SMEP/SMAP alone already
> eliminates a number of CR4 writes afaict.

Differing scenarios (SMEP/SMAP only makes a perf difference in practice
32bit PV-shim'd guests), but ISTR the TLB flushing side of things
dominating in terms of performance.

>> Depending on the view of other PV shim usability issues, 6dd95b02ea27
>> and f9dee1f945eb regarding vtsc make a large difference and should be
>> considered.
> I've queued the latter, but the former doesn't really look like
> backporting material to me.

It is a functional prerequisite.  The stats in the former are protected
by the lock which is removed by the latter.

An alternative (in theory) would be to change the stats to atomic64_t's,
except that we don't have any of that infrastructure.  Given that we
already considered deleting those stats several years ago (due to being
of dubious use to begin with), I told Igor not to waste time trying to
fix it differently.

>> 188f479de4b7 and 005de45c887e are both core scheduling bugfixes and
>> should be considered, even if they are a little too fresh right at the
>> moment.
> "Freshness" is not an issue imo. They've passed the push gate on
> master, and I wouldn't get around to apply them right away anyway.
> With these it's just the typical situation for features that are
> still "experimental" or alike - I'm never really certain whether
> it's better to pull in such fixes (and have more code churn) or
> to leave them out. And explicit request like this one of yours
> helps take a decision.

In this case, they are concrete bugfixes which were discovered, and
confirmed fixed, by XenServers testing on a 4.13 base.

My main purpose in highlighting them is simply that they don't get
missed for 4.13.1.  After all, there are still other blocking bugs with
core scheduling even on staging.

~Andrew

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

Re: [Xen-devel] Backports to 4.13

[Jan Beulich]

On 06.03.2020 15:31, Andrew Cooper wrote:
> On 06/03/2020 14:11, Jan Beulich wrote:
>> On 06.03.2020 14:52, Andrew Cooper wrote:
>>> Depending on the view of other PV shim usability issues, 6dd95b02ea27
>>> and f9dee1f945eb regarding vtsc make a large difference and should be
>>> considered.
>> I've queued the latter, but the former doesn't really look like
>> backporting material to me.
> 
> It is a functional prerequisite.  The stats in the former are protected
> by the lock which is removed by the latter.

Not really, no - the same stats also get update without that lock
in _hvm_rdtsc_intercept(). Plus in your patch description you
also state this: "(currently they left unprotected for HVM domains)".
It's debugging only code really, which I don't think does much harm
when left in.

> An alternative (in theory) would be to change the stats to atomic64_t's,
> except that we don't have any of that infrastructure.  Given that we
> already considered deleting those stats several years ago (due to being
> of dubious use to begin with), I told Igor not to waste time trying to
> fix it differently.

I can certainly see this as one valid way to approach this.

>>> 188f479de4b7 and 005de45c887e are both core scheduling bugfixes and
>>> should be considered, even if they are a little too fresh right at the
>>> moment.
>> "Freshness" is not an issue imo. They've passed the push gate on
>> master, and I wouldn't get around to apply them right away anyway.
>> With these it's just the typical situation for features that are
>> still "experimental" or alike - I'm never really certain whether
>> it's better to pull in such fixes (and have more code churn) or
>> to leave them out. And explicit request like this one of yours
>> helps take a decision.
> 
> In this case, they are concrete bugfixes which were discovered, and
> confirmed fixed, by XenServers testing on a 4.13 base.
> 
> My main purpose in highlighting them is simply that they don't get
> missed for 4.13.1.  After all, there are still other blocking bugs with
> core scheduling even on staging.

To be honest, them getting missed wouldn't be a problem as long as
we don't mean to move core scheduling out of its experimental state
for 4.13.1. Backporting such fixes really is optional, and I'd
likely stop doing so if the volume increased meaningfully.

Jan

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

Re: Backports to 4.13

[Ian Jackson]

Andrew Cooper writes ("Backports to 4.13"):
> On the tools side of things, f50a4f6e244c aafae0e800e9 2a62c22715bf
> d79cc6bc2bac 0729830cc425 are all bugs in CPUID and/or migration.  "Fix
> HVM_PARAM_PAE_ENABLED handling" is only for 4.13, whereas all the others
> are back to 4.7 (technically speaking).

Done.

There seem to have been quite few requests, but there were a lot of
fixes tagged with Backport or Fixes.  I have now applied these (not
just to 4.13 but to all the supported trees so back to 4.10, as
applicable; plus one non-security bugfix I considered important enough
to go to the security-supported 4.9).

Ian.

Re: Backports to 4.13

[Jan Beulich]

On 05.05.2020 16:51, Ian Jackson wrote:
> Andrew Cooper writes ("Backports to 4.13"):
>> On the tools side of things, f50a4f6e244c aafae0e800e9 2a62c22715bf
>> d79cc6bc2bac 0729830cc425 are all bugs in CPUID and/or migration.  "Fix
>> HVM_PARAM_PAE_ENABLED handling" is only for 4.13, whereas all the others
>> are back to 4.7 (technically speaking).
> 
> Done.
> 
> There seem to have been quite few requests, but there were a lot of
> fixes tagged with Backport or Fixes.  I have now applied these (not
> just to 4.13 but to all the supported trees so back to 4.10, as
> applicable; plus one non-security bugfix I considered important enough
> to go to the security-supported 4.9).

Why back to 4.10? Only 4.12 and 4.13 are fully supported at this point,
afaict. Older trees should only get security fixes imo, with extremely
few exceptions.

Jan