| Message ID | 20220909144552.3000716-1-quic_eberman@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] KVM: arm64: Only set KVM_MODE_PROTECTED if is_hyp_mode_available() | expand |
On Fri, Sep 09, 2022 at 07:45:52AM -0700, Elliot Berman wrote: > Do not switch kvm_mode to KVM_MODE_PROTECTED if hypervisor mode is not > available. This prevents "Protected KVM" cpu capability being reported > when Linux is booting in EL1 and would not have KVM enabled. > > Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> > --- > arch/arm64/kvm/arm.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 8fe73ee5fa84..861f4b388879 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -2272,7 +2272,9 @@ static int __init early_kvm_mode_cfg(char *arg) > return -EINVAL; > > if (strcmp(arg, "protected") == 0) { > - if (!is_kernel_in_hyp_mode()) > + if (!is_hyp_mode_available()) > + kvm_mode = KVM_MODE_DEFAULT; I think kvm_mode is already KVM_MODE_DEFAULT at this point. You may want to print a warning instead.
On 9/9/2022 10:28 AM, Catalin Marinas wrote: > On Fri, Sep 09, 2022 at 07:45:52AM -0700, Elliot Berman wrote: >> Do not switch kvm_mode to KVM_MODE_PROTECTED if hypervisor mode is not >> available. This prevents "Protected KVM" cpu capability being reported >> when Linux is booting in EL1 and would not have KVM enabled. >> >> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> >> --- >> arch/arm64/kvm/arm.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c >> index 8fe73ee5fa84..861f4b388879 100644 >> --- a/arch/arm64/kvm/arm.c >> +++ b/arch/arm64/kvm/arm.c >> @@ -2272,7 +2272,9 @@ static int __init early_kvm_mode_cfg(char *arg) >> return -EINVAL; >> >> if (strcmp(arg, "protected") == 0) { >> - if (!is_kernel_in_hyp_mode()) >> + if (!is_hyp_mode_available()) >> + kvm_mode = KVM_MODE_DEFAULT; > > I think kvm_mode is already KVM_MODE_DEFAULT at this point. You may want > to print a warning instead. > Does it make sense to print warning for kvm-arm.mode=nvhe as well?
On Fri, 09 Sep 2022 18:55:18 +0100, Elliot Berman <quic_eberman@quicinc.com> wrote: > > > > On 9/9/2022 10:28 AM, Catalin Marinas wrote: > > On Fri, Sep 09, 2022 at 07:45:52AM -0700, Elliot Berman wrote: > >> Do not switch kvm_mode to KVM_MODE_PROTECTED if hypervisor mode is not > >> available. This prevents "Protected KVM" cpu capability being reported > >> when Linux is booting in EL1 and would not have KVM enabled. > >> > >> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> > >> --- > >> arch/arm64/kvm/arm.c | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > >> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > >> index 8fe73ee5fa84..861f4b388879 100644 > >> --- a/arch/arm64/kvm/arm.c > >> +++ b/arch/arm64/kvm/arm.c > >> @@ -2272,7 +2272,9 @@ static int __init early_kvm_mode_cfg(char *arg) > >> return -EINVAL; > >> if (strcmp(arg, "protected") == 0) { > >> - if (!is_kernel_in_hyp_mode()) > >> + if (!is_hyp_mode_available()) > >> + kvm_mode = KVM_MODE_DEFAULT; > > > > I think kvm_mode is already KVM_MODE_DEFAULT at this point. You may want > > to print a warning instead. > > > > Does it make sense to print warning for kvm-arm.mode=nvhe as well? In general, specifying a kvm-arm.mode when no hypervisor mode is available should be reported as a warning. Thanks, M.
On Sat, Sep 10, 2022 at 10:09:31AM +0100, Marc Zyngier wrote: > On Fri, 09 Sep 2022 18:55:18 +0100, > Elliot Berman <quic_eberman@quicinc.com> wrote: > > > > > > > > On 9/9/2022 10:28 AM, Catalin Marinas wrote: > > > On Fri, Sep 09, 2022 at 07:45:52AM -0700, Elliot Berman wrote: > > >> Do not switch kvm_mode to KVM_MODE_PROTECTED if hypervisor mode is not > > >> available. This prevents "Protected KVM" cpu capability being reported > > >> when Linux is booting in EL1 and would not have KVM enabled. > > >> > > >> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> > > >> --- > > >> arch/arm64/kvm/arm.c | 4 +++- > > >> 1 file changed, 3 insertions(+), 1 deletion(-) > > >> > > >> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > > >> index 8fe73ee5fa84..861f4b388879 100644 > > >> --- a/arch/arm64/kvm/arm.c > > >> +++ b/arch/arm64/kvm/arm.c > > >> @@ -2272,7 +2272,9 @@ static int __init early_kvm_mode_cfg(char *arg) > > >> return -EINVAL; > > >> if (strcmp(arg, "protected") == 0) { > > >> - if (!is_kernel_in_hyp_mode()) > > >> + if (!is_hyp_mode_available()) > > >> + kvm_mode = KVM_MODE_DEFAULT; > > > > > > I think kvm_mode is already KVM_MODE_DEFAULT at this point. You may want > > > to print a warning instead. > > > > > > > Does it make sense to print warning for kvm-arm.mode=nvhe as well? > > In general, specifying a kvm-arm.mode when no hypervisor mode is > available should be reported as a warning. As long as this is pr_warn() rather than WARN() then I agree. Otherwise, kernels with a kvm-arm.mode hardcoded in CONFIG_CMDLINE (e.g. Android's GKI) will make for noisy guests. Will
On Sat, 10 Sep 2022 14:43:44 +0100, Will Deacon <will@kernel.org> wrote: > > On Sat, Sep 10, 2022 at 10:09:31AM +0100, Marc Zyngier wrote: > > On Fri, 09 Sep 2022 18:55:18 +0100, > > Elliot Berman <quic_eberman@quicinc.com> wrote: > > > > > > > > > > > > On 9/9/2022 10:28 AM, Catalin Marinas wrote: > > > > On Fri, Sep 09, 2022 at 07:45:52AM -0700, Elliot Berman wrote: > > > >> Do not switch kvm_mode to KVM_MODE_PROTECTED if hypervisor mode is not > > > >> available. This prevents "Protected KVM" cpu capability being reported > > > >> when Linux is booting in EL1 and would not have KVM enabled. > > > >> > > > >> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> > > > >> --- > > > >> arch/arm64/kvm/arm.c | 4 +++- > > > >> 1 file changed, 3 insertions(+), 1 deletion(-) > > > >> > > > >> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > > > >> index 8fe73ee5fa84..861f4b388879 100644 > > > >> --- a/arch/arm64/kvm/arm.c > > > >> +++ b/arch/arm64/kvm/arm.c > > > >> @@ -2272,7 +2272,9 @@ static int __init early_kvm_mode_cfg(char *arg) > > > >> return -EINVAL; > > > >> if (strcmp(arg, "protected") == 0) { > > > >> - if (!is_kernel_in_hyp_mode()) > > > >> + if (!is_hyp_mode_available()) > > > >> + kvm_mode = KVM_MODE_DEFAULT; > > > > > > > > I think kvm_mode is already KVM_MODE_DEFAULT at this point. You may want > > > > to print a warning instead. > > > > > > > > > > Does it make sense to print warning for kvm-arm.mode=nvhe as well? > > > > In general, specifying a kvm-arm.mode when no hypervisor mode is > > available should be reported as a warning. > > As long as this is pr_warn() rather than WARN() then I agree. Otherwise, > kernels with a kvm-arm.mode hardcoded in CONFIG_CMDLINE (e.g. Android's > GKI) will make for noisy guests. Indeed, pr_warn() is what I had in mind. A WARN() would be pretty overkill, as there is nothing majorly wrong with booting at EL1, just an impossibility to honour the request from the command line. M.
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 8fe73ee5fa84..861f4b388879 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -2272,7 +2272,9 @@ static int __init early_kvm_mode_cfg(char *arg) return -EINVAL; if (strcmp(arg, "protected") == 0) { - if (!is_kernel_in_hyp_mode()) + if (!is_hyp_mode_available()) + kvm_mode = KVM_MODE_DEFAULT; + else if (!is_kernel_in_hyp_mode()) kvm_mode = KVM_MODE_PROTECTED; else pr_warn_once("Protected KVM not available with VHE\n");
Do not switch kvm_mode to KVM_MODE_PROTECTED if hypervisor mode is not available. This prevents "Protected KVM" cpu capability being reported when Linux is booting in EL1 and would not have KVM enabled. Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> --- arch/arm64/kvm/arm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) base-commit: 0982c8d859f8f7022b9fd44d421c7ec721bb41f9