| Message ID | CAL_Jsq+vGd7+pw6YX0YNHO5rVSoiHCHbtXXdZ-3grnEcAOc38Q@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/14/2013 08:54 PM, Rob Herring wrote: > On Wed, Aug 14, 2013 at 4:22 AM, Andre Przywara > <andre.przywara@calxeda.com> wrote: >> Guest kernels with CONFIG_L2X0 set (for instance Highbank or OMAP4) >> will trigger SMCs to handle the L2 cache controller (PL310). >> This will currently inject #UNDEFs and eventually stop the guest. >> >> We don't need explicit L2 cache controller handling on A15s anymore, >> so it is safe to simply ignore these calls and proceed with the next >> instruction. >> >> Signed-off-by: Andre Przywara <andre.przywara@calxeda.com> >> --- >> arch/arm/kvm/handle_exit.c | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) > > At least for highbank, we can fix this in the kernel: Yes, and we should do. But that won't fix older guest kernels, say Ubuntu 12.10 or the like. And I think this is a use case for virtualization, so we need both, guest and host fix. Regards, Andre > > diff --git a/arch/arm/mach-highbank/highbank.c > b/arch/arm/mach-highbank/highbank.c > index 1894dcf..b5d0375 100644 > --- a/arch/arm/mach-highbank/highbank.c > +++ b/arch/arm/mach-highbank/highbank.c > @@ -77,8 +77,10 @@ static void __init highbank_init_irq(void) > { > irqchip_init(); > > - if (of_find_compatible_node(NULL, NULL, "arm,cortex-a9")) > - highbank_scu_map_io(); > + if (!of_find_compatible_node(NULL, NULL, "arm,cortex-a9")) > + return; > + > + highbank_scu_map_io(); > > #ifdef CONFIG_CACHE_L2X0 > /* Enable PL310 L2 Cache controller */ > >> >> diff --git a/arch/arm/kvm/handle_exit.c b/arch/arm/kvm/handle_exit.c >> index df4c82d..2cbe6a0 100644 >> --- a/arch/arm/kvm/handle_exit.c >> +++ b/arch/arm/kvm/handle_exit.c >> @@ -50,8 +50,28 @@ static int handle_hvc(struct kvm_vcpu *vcpu, struct kvm_run *run) >> return 1; >> } >> >> +/* >> + * OMAP4 and Highbank machines do a SMC call to handle the L2 cache >> + * controller. They put 0x102 in r12 to request this functionality. >> + * This is not needed on A15s, so we can safely ignore it in KVM guests. >> + */ >> +static int kvm_ignore_l2x0_call(struct kvm_vcpu *vcpu) >> +{ >> + unsigned long fn_nr = *vcpu_reg(vcpu, 12) & ~((u32) 0); >> + >> + if (fn_nr == 0x102) { >> + kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu)); >> + return 1; >> + } >> + >> + return 0; >> +} >> + >> static int handle_smc(struct kvm_vcpu *vcpu, struct kvm_run *run) >> { >> + if (kvm_ignore_l2x0_call(vcpu)) >> + return 1; >> + >> kvm_inject_undefined(vcpu); >> return 1; >> } >> -- >> 1.7.12.1 >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, Aug 14, 2013 at 10:20:03PM +0200, Andre Przywara wrote: > On 08/14/2013 08:54 PM, Rob Herring wrote: > >On Wed, Aug 14, 2013 at 4:22 AM, Andre Przywara > ><andre.przywara@calxeda.com> wrote: > >>Guest kernels with CONFIG_L2X0 set (for instance Highbank or OMAP4) > >>will trigger SMCs to handle the L2 cache controller (PL310). > >>This will currently inject #UNDEFs and eventually stop the guest. > >> > >>We don't need explicit L2 cache controller handling on A15s anymore, > >>so it is safe to simply ignore these calls and proceed with the next > >>instruction. > >> > >>Signed-off-by: Andre Przywara <andre.przywara@calxeda.com> > >>--- > >> arch/arm/kvm/handle_exit.c | 20 ++++++++++++++++++++ > >> 1 file changed, 20 insertions(+) > > > >At least for highbank, we can fix this in the kernel: > > Yes, and we should do. But that won't fix older guest kernels, say > Ubuntu 12.10 or the like. And I think this is a use case for > virtualization, so we need both, guest and host fix. > Agreed, but we need a more generic solution for the secure call handling. I've created a backlog item in Linaro's JIRA (CARD-801) for this work, let's see how quickly we can get it approved and put on the roadmap. -Christoffer
On 08/14/2013 10:43 PM, Christoffer Dall wrote: > On Wed, Aug 14, 2013 at 10:20:03PM +0200, Andre Przywara wrote: >> On 08/14/2013 08:54 PM, Rob Herring wrote: >>> On Wed, Aug 14, 2013 at 4:22 AM, Andre Przywara >>> <andre.przywara@calxeda.com> wrote: >>>> Guest kernels with CONFIG_L2X0 set (for instance Highbank or OMAP4) >>>> will trigger SMCs to handle the L2 cache controller (PL310). >>>> This will currently inject #UNDEFs and eventually stop the guest. >>>> >>>> We don't need explicit L2 cache controller handling on A15s anymore, >>>> so it is safe to simply ignore these calls and proceed with the next >>>> instruction. >>>> >>>> Signed-off-by: Andre Przywara <andre.przywara@calxeda.com> >>>> --- >>>> arch/arm/kvm/handle_exit.c | 20 ++++++++++++++++++++ >>>> 1 file changed, 20 insertions(+) >>> >>> At least for highbank, we can fix this in the kernel: >> >> Yes, and we should do. But that won't fix older guest kernels, say >> Ubuntu 12.10 or the like. And I think this is a use case for >> virtualization, so we need both, guest and host fix. >> > Agreed, but we need a more generic solution for the secure call > handling. I've created a backlog item in Linaro's JIRA (CARD-801) for > this work, let's see how quickly we can get it approved and put on the > roadmap. So I did some research already, I am not sure we can wait until Jira is ready ;-) I'd opt for something like this: 1. Allow userland to let the kernel ignore all smc's. That's low overhead, easy to implement and would cover Highbank and Broadcom, which do only L2 cache controller handling via smc. 2. Think about how to handle TI Keystone and Qualcomm MSM, which do secondary cores bringup via smc's. Do we need to support this or can we demand PSCI support? If I got this correctly, a PSCI node in the DTB overrides any platform smp_ops, so injecting PSCI should avoid those smc's on those two platforms. 3. Agree on whether we support PSCI via smc. I think we abandoned this with 24a7f67 (ARM: KVM: Don't handle PSCI calls via SMC), so do we really want to re-introduce it? 4. Dig through all this OMAP smc code to decide what we really want to emulate and whether we need to: Maybe we can safely ignore this since it is for OMAP4 with A9s or lower only. If there is a need to emulate, fold this into one ioctl which also enables the ignore-all case. I am not sure whether it is a wise decision to pull _all_ SMC handling unconditionally into userland, since that would separate the source of the SMCs (the kernel) and their emulation. Regards, Andre.
On Thu, Aug 15, 2013 at 12:05:01AM +0200, Andre Przywara wrote: > On 08/14/2013 10:43 PM, Christoffer Dall wrote: > >On Wed, Aug 14, 2013 at 10:20:03PM +0200, Andre Przywara wrote: > >>On 08/14/2013 08:54 PM, Rob Herring wrote: > >>>On Wed, Aug 14, 2013 at 4:22 AM, Andre Przywara > >>><andre.przywara@calxeda.com> wrote: > >>>>Guest kernels with CONFIG_L2X0 set (for instance Highbank or OMAP4) > >>>>will trigger SMCs to handle the L2 cache controller (PL310). > >>>>This will currently inject #UNDEFs and eventually stop the guest. > >>>> > >>>>We don't need explicit L2 cache controller handling on A15s anymore, > >>>>so it is safe to simply ignore these calls and proceed with the next > >>>>instruction. > >>>> > >>>>Signed-off-by: Andre Przywara <andre.przywara@calxeda.com> > >>>>--- > >>>> arch/arm/kvm/handle_exit.c | 20 ++++++++++++++++++++ > >>>> 1 file changed, 20 insertions(+) > >>> > >>>At least for highbank, we can fix this in the kernel: > >> > >>Yes, and we should do. But that won't fix older guest kernels, say > >>Ubuntu 12.10 or the like. And I think this is a use case for > >>virtualization, so we need both, guest and host fix. > >> > >Agreed, but we need a more generic solution for the secure call > >handling. I've created a backlog item in Linaro's JIRA (CARD-801) for > >this work, let's see how quickly we can get it approved and put on the > >roadmap. > > So I did some research already, I am not sure we can wait until Jira > is ready ;-) This is not a quick-and-dirty sort of fix, but something we need to fix properly. > I'd opt for something like this: I'm confused: are you giving us a list of choices or are you listing the work items that you think we need to do? > 1. Allow userland to let the kernel ignore all smc's. That's low > overhead, easy to implement and would cover Highbank and Broadcom, > which do only L2 cache controller handling via smc. Hmmm, it's low overhead, but we still need to come up with an API for this (perhaps it's a vcpu feature), but might be abused. Again, I think this would be solved in a more generic approach - see Peter's suggestion on the discussion of the patch. > 2. Think about how to handle TI Keystone and Qualcomm MSM, which do > secondary cores bringup via smc's. Do we need to support this or can > we demand PSCI support? If I got this correctly, a PSCI node in the > DTB overrides any platform smp_ops, so injecting PSCI should avoid > those smc's on those two platforms. Yes, and this is only relevant if you want to run those kinds of guests. If the dominating use case is mach-virt, isn't all of this sort of moot? > 3. Agree on whether we support PSCI via smc. I think we abandoned > this with 24a7f67 (ARM: KVM: Don't handle PSCI calls via SMC), so do > we really want to re-introduce it? I don't see any reason to introduce that right now, but I certainly don't want to build an infrastructure around SMC handling that makes it hard or impossible to handle guests that do PSCI calls. > 4. Dig through all this OMAP smc code to decide what we really want > to emulate and whether we need to: Maybe we can safely ignore this > since it is for OMAP4 with A9s or lower only. > If there is a need to emulate, fold this into one ioctl which also > enables the ignore-all case. > I don't want to dig through any OMAP smc code, no. Why are we talking about supporting these arcane guest kernels? Is this not only a problem for a few recent kernels that have multiplatform support and some other combination of configs enabled and disregard info in the DT? > I am not sure whether it is a wise decision to pull _all_ SMC > handling unconditionally into userland, since that would separate > the source of the SMCs (the kernel) and their emulation. > Hmmm... The source of the SMCs is the guest software, and the only reasons to keep it in the host kernel would be performance or the requirement to perform privileged operation on the host cpu to emulate the smc. The latter may be the case, but I want to get a clearer picture of which cases we're trying to support here exactly, and why. -Christoffer
On 14 August 2013 21:43, Christoffer Dall <christoffer.dall@linaro.org> wrote: > Agreed, but we need a more generic solution for the secure call > handling. I've created a backlog item in Linaro's JIRA (CARD-801) for > this work, let's see how quickly we can get it approved and put on the > roadmap. FYI, here's my sketch from last year of what I had in mind: http://lists.gnu.org/archive/html/qemu-devel/2012-05/msg03012.html -- PMM
diff --git a/arch/arm/mach-highbank/highbank.c b/arch/arm/mach-highbank/highbank.c index 1894dcf..b5d0375 100644 --- a/arch/arm/mach-highbank/highbank.c +++ b/arch/arm/mach-highbank/highbank.c @@ -77,8 +77,10 @@ static void __init highbank_init_irq(void) { irqchip_init(); - if (of_find_compatible_node(NULL, NULL, "arm,cortex-a9")) - highbank_scu_map_io(); + if (!of_find_compatible_node(NULL, NULL, "arm,cortex-a9")) + return; + + highbank_scu_map_io(); #ifdef CONFIG_CACHE_L2X0 /* Enable PL310 L2 Cache controller */