Re: [Xen-devel] [PATCH 10/34] x86: stub out has_* testing for emulation flags
On Mon, Aug 20, 2018 at 06:37:28AM -0600, Jan Beulich wrote: > >>> On 17.08.18 at 17:12, wrote: > > Most are all HVM only. Provide stubs for !CONFIG_HVM. > > > > One exception is PIT emulation, which is available to both PV and HVM. > > > > Signed-off-by: Wei Liu > > --- > > xen/include/asm-x86/domain.h | 24 +++- > > 1 file changed, 23 insertions(+), 1 deletion(-) > > > > diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h > > index 09f6b3d..4c18054 100644 > > --- a/xen/include/asm-x86/domain.h > > +++ b/xen/include/asm-x86/domain.h > > @@ -440,6 +440,8 @@ struct arch_domain > > uint32_t emulation_flags; > > } __cacheline_aligned; > > > > +#ifdef CONFIG_HVM > > + > > #define has_vlapic(d) (!!((d)->arch.emulation_flags & > > XEN_X86_EMU_LAPIC)) > > #define has_vhpet(d) (!!((d)->arch.emulation_flags & > > XEN_X86_EMU_HPET)) > > #define has_vpm(d) (!!((d)->arch.emulation_flags & XEN_X86_EMU_PM)) > > @@ -448,11 +450,31 @@ struct arch_domain > > #define has_vpic(d)(!!((d)->arch.emulation_flags & > > XEN_X86_EMU_PIC)) > > #define has_vvga(d)(!!((d)->arch.emulation_flags & > > XEN_X86_EMU_VGA)) > > #define has_viommu(d) (!!((d)->arch.emulation_flags & > > XEN_X86_EMU_IOMMU)) > > -#define has_vpit(d)(!!((d)->arch.emulation_flags & > > XEN_X86_EMU_PIT)) > > #define has_pirq(d)(!!((d)->arch.emulation_flags & \ > > XEN_X86_EMU_USE_PIRQ)) > > #define has_vpci(d)(!!((d)->arch.emulation_flags & > > XEN_X86_EMU_VPCI)) > > > > +#else > > + > > +#define has_vlapic(d) (0) > > +#define has_vhpet(d) (0) > > +#define has_vpm(d)(0) > > +#define has_vrtc(d) (0) > > +#define has_vioapic(d)(0) > > +#define has_vpic(d) (0) > > +#define has_vvga(d) (0) > > +#define has_viommu(d) (0) > > +#define has_pirq(d) (0) > > +#define has_vpci(d) (0) > > + > > +#endif > > I'm not convinced - this introduces disconnected duplicate logic > (between here and emulation_flags_ok()). If we were to go this > route, I think comments would need to be added on both sides, > each referring to the other one. > > Furthermore, if we go this route, then "false" please instead of > "(0)". This patch can be dropped with adjustment to other files. > > > +#if CONFIG_HVM || CONFIG_PV > > +#define has_vpit(d)(!!((d)->arch.emulation_flags & > > XEN_X86_EMU_PIT)) > > +#else > > +#define has_vpit(d) (0) > > +#endif > > What purpose would a Xen serve with both HVM and PV disabled? > IOW - I don't think any #if is warranted here. This is useful for developers. I would certainly build with both PV and HVM disabled in the future to catch any wrong assumptions that creep into x86 common code. (Though in this instance the addition here is not needed) Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 10/34] x86: stub out has_* testing for emulation flags
>>> On 17.08.18 at 17:12, wrote: > Most are all HVM only. Provide stubs for !CONFIG_HVM. > > One exception is PIT emulation, which is available to both PV and HVM. > > Signed-off-by: Wei Liu > --- > xen/include/asm-x86/domain.h | 24 +++- > 1 file changed, 23 insertions(+), 1 deletion(-) > > diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h > index 09f6b3d..4c18054 100644 > --- a/xen/include/asm-x86/domain.h > +++ b/xen/include/asm-x86/domain.h > @@ -440,6 +440,8 @@ struct arch_domain > uint32_t emulation_flags; > } __cacheline_aligned; > > +#ifdef CONFIG_HVM > + > #define has_vlapic(d) (!!((d)->arch.emulation_flags & > XEN_X86_EMU_LAPIC)) > #define has_vhpet(d) (!!((d)->arch.emulation_flags & XEN_X86_EMU_HPET)) > #define has_vpm(d) (!!((d)->arch.emulation_flags & XEN_X86_EMU_PM)) > @@ -448,11 +450,31 @@ struct arch_domain > #define has_vpic(d)(!!((d)->arch.emulation_flags & XEN_X86_EMU_PIC)) > #define has_vvga(d)(!!((d)->arch.emulation_flags & XEN_X86_EMU_VGA)) > #define has_viommu(d) (!!((d)->arch.emulation_flags & > XEN_X86_EMU_IOMMU)) > -#define has_vpit(d)(!!((d)->arch.emulation_flags & XEN_X86_EMU_PIT)) > #define has_pirq(d)(!!((d)->arch.emulation_flags & \ > XEN_X86_EMU_USE_PIRQ)) > #define has_vpci(d)(!!((d)->arch.emulation_flags & XEN_X86_EMU_VPCI)) > > +#else > + > +#define has_vlapic(d) (0) > +#define has_vhpet(d) (0) > +#define has_vpm(d)(0) > +#define has_vrtc(d) (0) > +#define has_vioapic(d)(0) > +#define has_vpic(d) (0) > +#define has_vvga(d) (0) > +#define has_viommu(d) (0) > +#define has_pirq(d) (0) > +#define has_vpci(d) (0) > + > +#endif I'm not convinced - this introduces disconnected duplicate logic (between here and emulation_flags_ok()). If we were to go this route, I think comments would need to be added on both sides, each referring to the other one. Furthermore, if we go this route, then "false" please instead of "(0)". > +#if CONFIG_HVM || CONFIG_PV > +#define has_vpit(d)(!!((d)->arch.emulation_flags & XEN_X86_EMU_PIT)) > +#else > +#define has_vpit(d) (0) > +#endif What purpose would a Xen serve with both HVM and PV disabled? IOW - I don't think any #if is warranted here. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 10/34] x86: stub out has_* testing for emulation flags
Most are all HVM only. Provide stubs for !CONFIG_HVM. One exception is PIT emulation, which is available to both PV and HVM. Signed-off-by: Wei Liu --- xen/include/asm-x86/domain.h | 24 +++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h index 09f6b3d..4c18054 100644 --- a/xen/include/asm-x86/domain.h +++ b/xen/include/asm-x86/domain.h @@ -440,6 +440,8 @@ struct arch_domain uint32_t emulation_flags; } __cacheline_aligned; +#ifdef CONFIG_HVM + #define has_vlapic(d) (!!((d)->arch.emulation_flags & XEN_X86_EMU_LAPIC)) #define has_vhpet(d) (!!((d)->arch.emulation_flags & XEN_X86_EMU_HPET)) #define has_vpm(d) (!!((d)->arch.emulation_flags & XEN_X86_EMU_PM)) @@ -448,11 +450,31 @@ struct arch_domain #define has_vpic(d)(!!((d)->arch.emulation_flags & XEN_X86_EMU_PIC)) #define has_vvga(d)(!!((d)->arch.emulation_flags & XEN_X86_EMU_VGA)) #define has_viommu(d) (!!((d)->arch.emulation_flags & XEN_X86_EMU_IOMMU)) -#define has_vpit(d)(!!((d)->arch.emulation_flags & XEN_X86_EMU_PIT)) #define has_pirq(d)(!!((d)->arch.emulation_flags & \ XEN_X86_EMU_USE_PIRQ)) #define has_vpci(d)(!!((d)->arch.emulation_flags & XEN_X86_EMU_VPCI)) +#else + +#define has_vlapic(d) (0) +#define has_vhpet(d) (0) +#define has_vpm(d)(0) +#define has_vrtc(d) (0) +#define has_vioapic(d)(0) +#define has_vpic(d) (0) +#define has_vvga(d) (0) +#define has_viommu(d) (0) +#define has_pirq(d) (0) +#define has_vpci(d) (0) + +#endif + +#if CONFIG_HVM || CONFIG_PV +#define has_vpit(d)(!!((d)->arch.emulation_flags & XEN_X86_EMU_PIT)) +#else +#define has_vpit(d) (0) +#endif + #define has_arch_pdevs(d)(!list_empty(&(d)->arch.pdev_list)) #define gdt_ldt_pt_idx(v) \ -- git-series 0.9.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
