[v2] x86/save: reserve HVM save record numbers that have been consumed...
diff mbox series

Message ID 20191219120455.13357-1-pdurrant@amazon.com
State Superseded
Headers show
Series
  • [v2] x86/save: reserve HVM save record numbers that have been consumed...
Related show

Commit Message

Paul Durrant Dec. 19, 2019, 12:04 p.m. UTC
...for patches not (yet) upstream.

This patch is simply adding a comment to reserve save record number space
to avoid the risk of clashes between existent downstream changes made by
Amazon and future upstream changes which may be incompatible.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Reviewed-by: Wei Liu <wl@xen.org>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>

v2:
 - Reduce patch to just a comment
---
 xen/include/public/arch-x86/hvm/save.h | 2 ++
 1 file changed, 2 insertions(+)

Comments

Andrew Cooper Dec. 19, 2019, 12:16 p.m. UTC | #1
On 19/12/2019 12:04, Paul Durrant wrote:
> ...for patches not (yet) upstream.
>
> This patch is simply adding a comment to reserve save record number space
> to avoid the risk of clashes between existent downstream changes made by
> Amazon and future upstream changes which may be incompatible.
>
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> Reviewed-by: Wei Liu <wl@xen.org>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: "Roger Pau Monné" <roger.pau@citrix.com>
>
> v2:
>  - Reduce patch to just a comment
> ---
>  xen/include/public/arch-x86/hvm/save.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h
> index b2ad3fcd74..b3d445acf7 100644
> --- a/xen/include/public/arch-x86/hvm/save.h
> +++ b/xen/include/public/arch-x86/hvm/save.h
> @@ -639,6 +639,8 @@ struct hvm_msr {
>  
>  #define CPU_MSR_CODE  20
>  
> +/* Range 22 - 40 reserved for Amazon */

What about 21 and 22?  And where does the actual number stop, because
based on v1, its not 40.

~Andrew
Paul Durrant Dec. 19, 2019, 12:37 p.m. UTC | #2
> -----Original Message-----
> From: Andrew Cooper <andrew.cooper3@citrix.com>
> Sent: 19 December 2019 12:16
> To: Durrant, Paul <pdurrant@amazon.com>; xen-devel@lists.xenproject.org
> Cc: Wei Liu <wl@xen.org>; Jan Beulich <jbeulich@suse.com>; Roger Pau Monné
> <roger.pau@citrix.com>
> Subject: Re: [PATCH v2] x86/save: reserve HVM save record numbers that
> have been consumed...
> 
> On 19/12/2019 12:04, Paul Durrant wrote:
> > ...for patches not (yet) upstream.
> >
> > This patch is simply adding a comment to reserve save record number
> space
> > to avoid the risk of clashes between existent downstream changes made by
> > Amazon and future upstream changes which may be incompatible.
> >
> > Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> > Reviewed-by: Wei Liu <wl@xen.org>
> > ---
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > Cc: "Roger Pau Monné" <roger.pau@citrix.com>
> >
> > v2:
> >  - Reduce patch to just a comment
> > ---
> >  xen/include/public/arch-x86/hvm/save.h | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/xen/include/public/arch-x86/hvm/save.h
> b/xen/include/public/arch-x86/hvm/save.h
> > index b2ad3fcd74..b3d445acf7 100644
> > --- a/xen/include/public/arch-x86/hvm/save.h
> > +++ b/xen/include/public/arch-x86/hvm/save.h
> > @@ -639,6 +639,8 @@ struct hvm_msr {
> >
> >  #define CPU_MSR_CODE  20
> >
> > +/* Range 22 - 40 reserved for Amazon */
> 
> What about 21 and 22?  And where does the actual number stop, because
> based on v1, its not 40.
> 

The range is inclusive (maybe that's not obvious?). For some reason 21 was skipped but why do you say the top is not 40? That was what I set HVM_SAVE_CODE_MAX to in v1.

  Paul
Andrew Cooper Dec. 19, 2019, 1:08 p.m. UTC | #3
On 19/12/2019 12:37, Durrant, Paul wrote:
>> -----Original Message-----
>> From: Andrew Cooper <andrew.cooper3@citrix.com>
>> Sent: 19 December 2019 12:16
>> To: Durrant, Paul <pdurrant@amazon.com>; xen-devel@lists.xenproject.org
>> Cc: Wei Liu <wl@xen.org>; Jan Beulich <jbeulich@suse.com>; Roger Pau Monné
>> <roger.pau@citrix.com>
>> Subject: Re: [PATCH v2] x86/save: reserve HVM save record numbers that
>> have been consumed...
>>
>> On 19/12/2019 12:04, Paul Durrant wrote:
>>> ...for patches not (yet) upstream.
>>>
>>> This patch is simply adding a comment to reserve save record number
>> space
>>> to avoid the risk of clashes between existent downstream changes made by
>>> Amazon and future upstream changes which may be incompatible.
>>>
>>> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
>>> Reviewed-by: Wei Liu <wl@xen.org>
>>> ---
>>> Cc: Jan Beulich <jbeulich@suse.com>
>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>>> Cc: "Roger Pau Monné" <roger.pau@citrix.com>
>>>
>>> v2:
>>>  - Reduce patch to just a comment
>>> ---
>>>  xen/include/public/arch-x86/hvm/save.h | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/xen/include/public/arch-x86/hvm/save.h
>> b/xen/include/public/arch-x86/hvm/save.h
>>> index b2ad3fcd74..b3d445acf7 100644
>>> --- a/xen/include/public/arch-x86/hvm/save.h
>>> +++ b/xen/include/public/arch-x86/hvm/save.h
>>> @@ -639,6 +639,8 @@ struct hvm_msr {
>>>
>>>  #define CPU_MSR_CODE  20
>>>
>>> +/* Range 22 - 40 reserved for Amazon */
>> What about 21 and 22?  And where does the actual number stop, because
>> based on v1, its not 40.
>>
> The range is inclusive (maybe that's not obvious?). For some reason 21 was skipped but why do you say the top is not 40? That was what I set HVM_SAVE_CODE_MAX to in v1.

You also said that included prospective headroom, which definitely isn't
fair to reserve for ABI breakage reasons.

Which numbers have actually been allocated?

~Andrew
Paul Durrant Dec. 19, 2019, 1:15 p.m. UTC | #4
> -----Original Message-----
> From: Andrew Cooper <andrew.cooper3@citrix.com>
> Sent: 19 December 2019 13:08
> To: Durrant, Paul <pdurrant@amazon.com>; xen-devel@lists.xenproject.org
> Cc: Wei Liu <wl@xen.org>; Jan Beulich <jbeulich@suse.com>; Roger Pau Monné
> <roger.pau@citrix.com>
> Subject: Re: [PATCH v2] x86/save: reserve HVM save record numbers that
> have been consumed...
> 
> On 19/12/2019 12:37, Durrant, Paul wrote:
> >> -----Original Message-----
> >> From: Andrew Cooper <andrew.cooper3@citrix.com>
> >> Sent: 19 December 2019 12:16
> >> To: Durrant, Paul <pdurrant@amazon.com>; xen-devel@lists.xenproject.org
> >> Cc: Wei Liu <wl@xen.org>; Jan Beulich <jbeulich@suse.com>; Roger Pau
> Monné
> >> <roger.pau@citrix.com>
> >> Subject: Re: [PATCH v2] x86/save: reserve HVM save record numbers that
> >> have been consumed...
> >>
> >> On 19/12/2019 12:04, Paul Durrant wrote:
> >>> ...for patches not (yet) upstream.
> >>>
> >>> This patch is simply adding a comment to reserve save record number
> >> space
> >>> to avoid the risk of clashes between existent downstream changes made
> by
> >>> Amazon and future upstream changes which may be incompatible.
> >>>
> >>> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> >>> Reviewed-by: Wei Liu <wl@xen.org>
> >>> ---
> >>> Cc: Jan Beulich <jbeulich@suse.com>
> >>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> >>> Cc: "Roger Pau Monné" <roger.pau@citrix.com>
> >>>
> >>> v2:
> >>>  - Reduce patch to just a comment
> >>> ---
> >>>  xen/include/public/arch-x86/hvm/save.h | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/xen/include/public/arch-x86/hvm/save.h
> >> b/xen/include/public/arch-x86/hvm/save.h
> >>> index b2ad3fcd74..b3d445acf7 100644
> >>> --- a/xen/include/public/arch-x86/hvm/save.h
> >>> +++ b/xen/include/public/arch-x86/hvm/save.h
> >>> @@ -639,6 +639,8 @@ struct hvm_msr {
> >>>
> >>>  #define CPU_MSR_CODE  20
> >>>
> >>> +/* Range 22 - 40 reserved for Amazon */
> >> What about 21 and 22?  And where does the actual number stop, because
> >> based on v1, its not 40.
> >>
> > The range is inclusive (maybe that's not obvious?). For some reason 21
> was skipped but why do you say the top is not 40? That was what I set
> HVM_SAVE_CODE_MAX to in v1.
> 
> You also said that included prospective headroom, which definitely isn't
> fair to reserve for ABI breakage reasons.
> 
> Which numbers have actually been allocated?
> 

The problem is that I don't yet know for sure. I have definitely seen patches using 22 thru 34. It is *probably* safe to restrict to that but does it really cost that much more to reserve some extra space just in case? I.e. if someone else adds 41 vs. 35 is it going to make much of a difference?

  Paul
Jan Beulich Dec. 19, 2019, 1:25 p.m. UTC | #5
On 19.12.2019 14:15, Durrant, Paul wrote:
>> From: Andrew Cooper <andrew.cooper3@citrix.com>
>> Sent: 19 December 2019 13:08
>>
>> On 19/12/2019 12:37, Durrant, Paul wrote:
>>>> From: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> Sent: 19 December 2019 12:16
>>>>
>>>> On 19/12/2019 12:04, Paul Durrant wrote:
>>>>> --- a/xen/include/public/arch-x86/hvm/save.h
>>>>> +++ b/xen/include/public/arch-x86/hvm/save.h
>>>>> @@ -639,6 +639,8 @@ struct hvm_msr {
>>>>>
>>>>>  #define CPU_MSR_CODE  20
>>>>>
>>>>> +/* Range 22 - 40 reserved for Amazon */
>>>> What about 21 and 22?  And where does the actual number stop, because
>>>> based on v1, its not 40.
>>>>
>>> The range is inclusive (maybe that's not obvious?). For some reason 21
>> was skipped but why do you say the top is not 40? That was what I set
>> HVM_SAVE_CODE_MAX to in v1.
>>
>> You also said that included prospective headroom, which definitely isn't
>> fair to reserve for ABI breakage reasons.
>>
>> Which numbers have actually been allocated?
>>
> 
> The problem is that I don't yet know for sure. I have definitely seen
> patches using 22 thru 34. It is *probably* safe to restrict to that but
> does it really cost that much more to reserve some extra space just in
> case? I.e. if someone else adds 41 vs. 35 is it going to make much of a
> difference?

Not _that much_, but still - it's a bodge, so let's try to limit it as
much as we can.

Jan
Paul Durrant Dec. 19, 2019, 1:27 p.m. UTC | #6
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 19 December 2019 13:25
> To: Durrant, Paul <pdurrant@amazon.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>; xen-
> devel@lists.xenproject.org; Wei Liu <wl@xen.org>; Roger Pau Monné
> <roger.pau@citrix.com>
> Subject: Re: [PATCH v2] x86/save: reserve HVM save record numbers that
> have been consumed...
> 
> On 19.12.2019 14:15, Durrant, Paul wrote:
> >> From: Andrew Cooper <andrew.cooper3@citrix.com>
> >> Sent: 19 December 2019 13:08
> >>
> >> On 19/12/2019 12:37, Durrant, Paul wrote:
> >>>> From: Andrew Cooper <andrew.cooper3@citrix.com>
> >>>> Sent: 19 December 2019 12:16
> >>>>
> >>>> On 19/12/2019 12:04, Paul Durrant wrote:
> >>>>> --- a/xen/include/public/arch-x86/hvm/save.h
> >>>>> +++ b/xen/include/public/arch-x86/hvm/save.h
> >>>>> @@ -639,6 +639,8 @@ struct hvm_msr {
> >>>>>
> >>>>>  #define CPU_MSR_CODE  20
> >>>>>
> >>>>> +/* Range 22 - 40 reserved for Amazon */
> >>>> What about 21 and 22?  And where does the actual number stop, because
> >>>> based on v1, its not 40.
> >>>>
> >>> The range is inclusive (maybe that's not obvious?). For some reason 21
> >> was skipped but why do you say the top is not 40? That was what I set
> >> HVM_SAVE_CODE_MAX to in v1.
> >>
> >> You also said that included prospective headroom, which definitely
> isn't
> >> fair to reserve for ABI breakage reasons.
> >>
> >> Which numbers have actually been allocated?
> >>
> >
> > The problem is that I don't yet know for sure. I have definitely seen
> > patches using 22 thru 34. It is *probably* safe to restrict to that but
> > does it really cost that much more to reserve some extra space just in
> > case? I.e. if someone else adds 41 vs. 35 is it going to make much of a
> > difference?
> 
> Not _that much_, but still - it's a bodge, so let's try to limit it as
> much as we can.
> 

OK, I'll send a v3 using 34 as the limit.

  Paul

> Jan

Patch
diff mbox series

diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h
index b2ad3fcd74..b3d445acf7 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -639,6 +639,8 @@  struct hvm_msr {
 
 #define CPU_MSR_CODE  20
 
+/* Range 22 - 40 reserved for Amazon */
+
 /*
  * Largest type-code in use
  */