Messages in this thread | | | Subject | Re: [PATCH] x86/hyperv: Fix hv_get/set_register for nested bringup | From | Jinank Jain <> | Date | Wed, 15 Feb 2023 22:05:29 +0530 |
| |
The patch looks good to me, apart from the comments from Wei regarding styling.
On 2/13/2023 7:58 PM, Wei Liu wrote: > A few comments on style. > > On Thu, Feb 09, 2023 at 02:02:52PM -0800, Nuno Das Neves wrote: >> hv_get_nested_reg only translates SINT0, resulting in the wrong sint >> being registered by nested vmbus. > Please put a blank line between paragraphs. > >> Fix the issue with new utility function hv_is_sint_reg. >> While at it, improve clarity of hv_set_non_nested_register and hv_is_synic_reg. >> >> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com> >> --- >> arch/x86/include/asm/mshyperv.h | 11 +++++++---- >> arch/x86/kernel/cpu/mshyperv.c | 8 ++++---- >> 2 files changed, 11 insertions(+), 8 deletions(-) >> >> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h >> index 9ae1a344536b..684c547c1cca 100644 >> --- a/arch/x86/include/asm/mshyperv.h >> +++ b/arch/x86/include/asm/mshyperv.h >> @@ -225,10 +225,13 @@ extern bool hv_isolation_type_snp(void); >> >> static inline bool hv_is_synic_reg(unsigned int reg) >> { >> - if ((reg >= HV_REGISTER_SCONTROL) && >> - (reg <= HV_REGISTER_SINT15)) >> - return true; >> - return false; >> + return (reg >= HV_REGISTER_SCONTROL) && >> + (reg <= HV_REGISTER_SINT15); >> +} > Please put a new line here. > > I can fix these issues too if you don't end up sending a new version due > to other issues. > > Jinank, please take a look. The code looks sensible to me, but I would > like you to have a look too. > > Thanks, > Wei. > >> +static inline bool hv_is_sint_reg(unsigned int reg) >> +{ >> + return (reg >= HV_REGISTER_SINT0) && >> + (reg <= HV_REGISTER_SINT15); >> } >> >> u64 hv_get_register(unsigned int reg); >> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c >> index 0ceb6d1f9c3c..6bd344e1200f 100644 >> --- a/arch/x86/kernel/cpu/mshyperv.c >> +++ b/arch/x86/kernel/cpu/mshyperv.c >> @@ -44,6 +44,9 @@ struct ms_hyperv_info ms_hyperv; >> #if IS_ENABLED(CONFIG_HYPERV) >> static inline unsigned int hv_get_nested_reg(unsigned int reg) >> { >> + if (hv_is_sint_reg(reg)) >> + return reg - HV_REGISTER_SINT0 + HV_REGISTER_NESTED_SINT0; >> + >> switch (reg) { >> case HV_REGISTER_SIMP: >> return HV_REGISTER_NESTED_SIMP; >> @@ -53,8 +56,6 @@ static inline unsigned int hv_get_nested_reg(unsigned int reg) >> return HV_REGISTER_NESTED_SVERSION; >> case HV_REGISTER_SCONTROL: >> return HV_REGISTER_NESTED_SCONTROL; >> - case HV_REGISTER_SINT0: >> - return HV_REGISTER_NESTED_SINT0; >> case HV_REGISTER_EOM: >> return HV_REGISTER_NESTED_EOM; >> default: >> @@ -80,8 +81,7 @@ void hv_set_non_nested_register(unsigned int reg, u64 value) >> hv_ghcb_msr_write(reg, value); >> >> /* Write proxy bit via wrmsl instruction */ >> - if (reg >= HV_REGISTER_SINT0 && >> - reg <= HV_REGISTER_SINT15) >> + if (hv_is_sint_reg(reg)) >> wrmsrl(reg, value | 1 << 20); >> } else { >> wrmsrl(reg, value); >> -- >> 2.25.1
Reviewed-by: Jinank Jain <jinankjain@linux.microsoft.com>
| |