| Message ID | 20191219120455.13357-1-pdurrant@amazon.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] x86/save: reserve HVM save record numbers that have been consumed... | expand |
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
> -----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
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
> -----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
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
> -----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
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 */