| Message ID | 20190723213553.22300-2-julien.grall@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: Xen hardening for newer Armv8 | expand |
Hi Julien, Julien Grall writes: > Currently, the structure vcpu_guest_core_regs is part of the public API. > This implies that any change in the structure should be backward > compatible. > > However, the structure is only needed by the tools and Xen. It is also > not expected to be ever used outside of that context. So we could save us > some headache by only declaring the structure for Xen and tools. > > Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Julien Grall <julien.grall@arm.com> > --- > This is a follow-up of the discussion [1]. > > [1] <3c245c5b-51c6-1d0e-ad6c-42414573166f@arm.com> > > Changes in v3: > - Avoid introduce a new #ifdef in the header by moving the > definitions later on. > --- > xen/include/public/arch-arm.h | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h > index 3e8cdc151d..7ce139a0f5 100644 > --- a/xen/include/public/arch-arm.h > +++ b/xen/include/public/arch-arm.h > @@ -197,6 +197,18 @@ > } while ( 0 ) > #define set_xen_guest_handle(hnd, val) set_xen_guest_handle_raw(hnd, val) > > +typedef uint64_t xen_pfn_t; > +#define PRI_xen_pfn PRIx64 > +#define PRIu_xen_pfn PRIu64 > + > +/* Maximum number of virtual CPUs in legacy multi-processor guests. */ > +/* Only one. All other VCPUS must use VCPUOP_register_vcpu_info */ Just a suggestion: you already touching this part. Maybe you'll fix this comment as well? > +#define XEN_LEGACY_MAX_VCPUS 1 > + > +typedef uint64_t xen_ulong_t; > +#define PRI_xen_ulong PRIx64 > + > +#if defined(__XEN__) || defined(__XEN_TOOLS__) > #if defined(__GNUC__) && !defined(__STRICT_ANSI__) > /* Anonymous union includes both 32- and 64-bit names (e.g., r0/x0). */ > # define __DECL_REG(n64, n32) union { \ > @@ -272,18 +284,6 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_guest_core_regs_t); > > #undef __DECL_REG > > -typedef uint64_t xen_pfn_t; > -#define PRI_xen_pfn PRIx64 > -#define PRIu_xen_pfn PRIu64 > - > -/* Maximum number of virtual CPUs in legacy multi-processor guests. */ > -/* Only one. All other VCPUS must use VCPUOP_register_vcpu_info */ > -#define XEN_LEGACY_MAX_VCPUS 1 > - > -typedef uint64_t xen_ulong_t; > -#define PRI_xen_ulong PRIx64 > - > -#if defined(__XEN__) || defined(__XEN_TOOLS__) > struct vcpu_guest_context { > #define _VGCF_online 0 > #define VGCF_online (1<<_VGCF_online)
On 26/07/2019 13:14, Volodymyr Babchuk wrote: > > Hi Julien, Hi Volodymyr, > Julien Grall writes: > >> Currently, the structure vcpu_guest_core_regs is part of the public API. >> This implies that any change in the structure should be backward >> compatible. >> >> However, the structure is only needed by the tools and Xen. It is also >> not expected to be ever used outside of that context. So we could save us >> some headache by only declaring the structure for Xen and tools. >> >> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> >> Signed-off-by: Julien Grall <julien.grall@arm.com> >> --- >> This is a follow-up of the discussion [1]. >> >> [1] <3c245c5b-51c6-1d0e-ad6c-42414573166f@arm.com> >> >> Changes in v3: >> - Avoid introduce a new #ifdef in the header by moving the >> definitions later on. >> --- >> xen/include/public/arch-arm.h | 24 ++++++++++++------------ >> 1 file changed, 12 insertions(+), 12 deletions(-) >> >> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h >> index 3e8cdc151d..7ce139a0f5 100644 >> --- a/xen/include/public/arch-arm.h >> +++ b/xen/include/public/arch-arm.h >> @@ -197,6 +197,18 @@ >> } while ( 0 ) >> #define set_xen_guest_handle(hnd, val) set_xen_guest_handle_raw(hnd, val) >> >> +typedef uint64_t xen_pfn_t; >> +#define PRI_xen_pfn PRIx64 >> +#define PRIu_xen_pfn PRIu64 >> + >> +/* Maximum number of virtual CPUs in legacy multi-processor guests. */ >> +/* Only one. All other VCPUS must use VCPUOP_register_vcpu_info */ > Just a suggestion: you already touching this part. Maybe you'll fix this > comment as well? I am not sure what's wrong with the current comment. Can you expand your thoughts please? Cheers,
Julien Grall writes: > On 26/07/2019 13:14, Volodymyr Babchuk wrote: >> >> Hi Julien, > > Hi Volodymyr, > >> Julien Grall writes: >> >>> Currently, the structure vcpu_guest_core_regs is part of the public API. >>> This implies that any change in the structure should be backward >>> compatible. >>> >>> However, the structure is only needed by the tools and Xen. It is also >>> not expected to be ever used outside of that context. So we could save us >>> some headache by only declaring the structure for Xen and tools. >>> >>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> >>> Signed-off-by: Julien Grall <julien.grall@arm.com> >>> --- >>> This is a follow-up of the discussion [1]. >>> >>> [1] <3c245c5b-51c6-1d0e-ad6c-42414573166f@arm.com> >>> >>> Changes in v3: >>> - Avoid introduce a new #ifdef in the header by moving the >>> definitions later on. >>> --- >>> xen/include/public/arch-arm.h | 24 ++++++++++++------------ >>> 1 file changed, 12 insertions(+), 12 deletions(-) >>> >>> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h >>> index 3e8cdc151d..7ce139a0f5 100644 >>> --- a/xen/include/public/arch-arm.h >>> +++ b/xen/include/public/arch-arm.h >>> @@ -197,6 +197,18 @@ >>> } while ( 0 ) >>> #define set_xen_guest_handle(hnd, val) set_xen_guest_handle_raw(hnd, val) >>> +typedef uint64_t xen_pfn_t; >>> +#define PRI_xen_pfn PRIx64 >>> +#define PRIu_xen_pfn PRIu64 >>> + >>> +/* Maximum number of virtual CPUs in legacy multi-processor guests. */ >>> +/* Only one. All other VCPUS must use VCPUOP_register_vcpu_info */ >> Just a suggestion: you already touching this part. Maybe you'll fix this >> comment as well? > > I am not sure what's wrong with the current comment. Can you expand > your thoughts please? Sure. It does not conform to CODING_STYLE: Comments containing a single sentence may end with a full stop; comments containing several sentences must have a full stop after each sentence. The second comment misses full stop at the end. Also, maybe we should consider this as s multi-line comment: Multi-line comment blocks should start and end with comment markers on separate lines and each line should begin with a leading '*'.
On Fri, 26 Jul 2019, Volodymyr Babchuk wrote: > > On 26/07/2019 13:14, Volodymyr Babchuk wrote: > >> > >> Hi Julien, > > > > Hi Volodymyr, > > > >> Julien Grall writes: > >> > >>> Currently, the structure vcpu_guest_core_regs is part of the public API. > >>> This implies that any change in the structure should be backward > >>> compatible. > >>> > >>> However, the structure is only needed by the tools and Xen. It is also > >>> not expected to be ever used outside of that context. So we could save us > >>> some headache by only declaring the structure for Xen and tools. > >>> > >>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> > >>> Signed-off-by: Julien Grall <julien.grall@arm.com> > >>> --- > >>> This is a follow-up of the discussion [1]. > >>> > >>> [1] <3c245c5b-51c6-1d0e-ad6c-42414573166f@arm.com> > >>> > >>> Changes in v3: > >>> - Avoid introduce a new #ifdef in the header by moving the > >>> definitions later on. > >>> --- > >>> xen/include/public/arch-arm.h | 24 ++++++++++++------------ > >>> 1 file changed, 12 insertions(+), 12 deletions(-) > >>> > >>> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h > >>> index 3e8cdc151d..7ce139a0f5 100644 > >>> --- a/xen/include/public/arch-arm.h > >>> +++ b/xen/include/public/arch-arm.h > >>> @@ -197,6 +197,18 @@ > >>> } while ( 0 ) > >>> #define set_xen_guest_handle(hnd, val) set_xen_guest_handle_raw(hnd, val) > >>> +typedef uint64_t xen_pfn_t; > >>> +#define PRI_xen_pfn PRIx64 > >>> +#define PRIu_xen_pfn PRIu64 > >>> + > >>> +/* Maximum number of virtual CPUs in legacy multi-processor guests. */ > >>> +/* Only one. All other VCPUS must use VCPUOP_register_vcpu_info */ > >> Just a suggestion: you already touching this part. Maybe you'll fix this > >> comment as well? > > > > I am not sure what's wrong with the current comment. Can you expand > > your thoughts please? > Sure. It does not conform to CODING_STYLE: > > Comments containing a single sentence may end with a full > stop; comments containing several sentences must have a full stop > after each sentence. > > The second comment misses full stop at the end. Also, maybe we should > consider this as s multi-line comment: > > Multi-line comment blocks should start and end with comment markers on > separate lines and each line should begin with a leading '*'. I'll improve on commit Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h index 3e8cdc151d..7ce139a0f5 100644 --- a/xen/include/public/arch-arm.h +++ b/xen/include/public/arch-arm.h @@ -197,6 +197,18 @@ } while ( 0 ) #define set_xen_guest_handle(hnd, val) set_xen_guest_handle_raw(hnd, val) +typedef uint64_t xen_pfn_t; +#define PRI_xen_pfn PRIx64 +#define PRIu_xen_pfn PRIu64 + +/* Maximum number of virtual CPUs in legacy multi-processor guests. */ +/* Only one. All other VCPUS must use VCPUOP_register_vcpu_info */ +#define XEN_LEGACY_MAX_VCPUS 1 + +typedef uint64_t xen_ulong_t; +#define PRI_xen_ulong PRIx64 + +#if defined(__XEN__) || defined(__XEN_TOOLS__) #if defined(__GNUC__) && !defined(__STRICT_ANSI__) /* Anonymous union includes both 32- and 64-bit names (e.g., r0/x0). */ # define __DECL_REG(n64, n32) union { \ @@ -272,18 +284,6 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_guest_core_regs_t); #undef __DECL_REG -typedef uint64_t xen_pfn_t; -#define PRI_xen_pfn PRIx64 -#define PRIu_xen_pfn PRIu64 - -/* Maximum number of virtual CPUs in legacy multi-processor guests. */ -/* Only one. All other VCPUS must use VCPUOP_register_vcpu_info */ -#define XEN_LEGACY_MAX_VCPUS 1 - -typedef uint64_t xen_ulong_t; -#define PRI_xen_ulong PRIx64 - -#if defined(__XEN__) || defined(__XEN_TOOLS__) struct vcpu_guest_context { #define _VGCF_online 0 #define VGCF_online (1<<_VGCF_online)
Currently, the structure vcpu_guest_core_regs is part of the public API. This implies that any change in the structure should be backward compatible. However, the structure is only needed by the tools and Xen. It is also not expected to be ever used outside of that context. So we could save us some headache by only declaring the structure for Xen and tools. Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Julien Grall <julien.grall@arm.com> --- This is a follow-up of the discussion [1]. [1] <3c245c5b-51c6-1d0e-ad6c-42414573166f@arm.com> Changes in v3: - Avoid introduce a new #ifdef in the header by moving the definitions later on. --- xen/include/public/arch-arm.h | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)