[RFC PATCH 2/2] hw/arm/raspi: Restrict BCM2835 / BCM2836 SoC to TCG

Philippe Mathieu-Daudé posted 2 patches 5 years ago
Maintainers: Michael Tokarev <mjt@tls.msk.ru>, Laurent Vivier <laurent@vivier.eu>
[RFC PATCH 2/2] hw/arm/raspi: Restrict BCM2835 / BCM2836 SoC to TCG
Posted by Philippe Mathieu-Daudé 5 years ago
KVM requires the target cpu to be at least ARMv8 architecture
(support on ARMv7 has been dropped in commit 82bf7ae84ce:
"target/arm: Remove KVM support for 32-bit Arm hosts").

From the various SoC used by the Raspberry Pi machines, only
the BCM2837 is an ARMv8 (Cortex-A53).

Restrict the BCM2835 (ARM1176) and BCM2836 (Cortex-A7) to TCG.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/bcm2836.c | 6 ++++++
 hw/arm/raspi.c   | 4 ++++
 2 files changed, 10 insertions(+)

diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index fd16ed87c40..3051764f2dc 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -89,6 +89,7 @@ static bool bcm283x_common_realize(DeviceState *dev, Error **errp)
     return true;
 }
 
+#ifdef CONFIG_TCG
 static void bcm2835_realize(DeviceState *dev, Error **errp)
 {
     BCM283XState *s = BCM283X(dev);
@@ -107,6 +108,7 @@ static void bcm2835_realize(DeviceState *dev, Error **errp)
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 1,
             qdev_get_gpio_in(DEVICE(&s->cpu[0].core), ARM_CPU_FIQ));
 }
+#endif /* CONFIG_TCG */
 
 static void bcm2836_realize(DeviceState *dev, Error **errp)
 {
@@ -178,6 +180,7 @@ static void bcm283x_class_init(ObjectClass *oc, void *data)
     dc->user_creatable = false;
 }
 
+#ifdef CONFIG_TCG
 static void bcm2835_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
@@ -201,6 +204,7 @@ static void bcm2836_class_init(ObjectClass *oc, void *data)
     bc->clusterid = 0xf;
     dc->realize = bcm2836_realize;
 };
+#endif /* CONFIG_TCG */
 
 #ifdef TARGET_AARCH64
 static void bcm2837_class_init(ObjectClass *oc, void *data)
@@ -227,6 +231,7 @@ static const TypeInfo bcm283x_types[] = {
         .class_init     = bcm283x_class_init,
         .abstract       = true,
     },
+#ifdef CONFIG_TCG
     {
         .name           = TYPE_BCM2835,
         .parent         = TYPE_BCM283X,
@@ -236,6 +241,7 @@ static const TypeInfo bcm283x_types[] = {
         .parent         = TYPE_BCM283X,
         .class_init     = bcm2836_class_init,
     },
+#endif /* CONFIG_TCG */
 #ifdef TARGET_AARCH64
     {
         .name           = TYPE_BCM2837,
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index dce966a4dd8..cfa15504d9c 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -319,6 +319,7 @@ static void raspi_machine_class_common_init(MachineClass *mc,
     mc->default_ram_id = "ram";
 };
 
+#ifdef CONFIG_TCG
 static void raspi0_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -346,6 +347,7 @@ static void raspi2b_machine_class_init(ObjectClass *oc, void *data)
     rmc->board_rev = 0xa21041;
     raspi_machine_class_common_init(mc, rmc->board_rev);
 };
+#endif /* CONFIG_TCG */
 
 #ifdef TARGET_AARCH64
 static void raspi3ap_machine_class_init(ObjectClass *oc, void *data)
@@ -376,6 +378,7 @@ static const TypeInfo raspi_machine_types[] = {
         .class_size     = sizeof(RaspiMachineClass),
         .abstract       = true,
     },
+#ifdef CONFIG_TCG
     {
         .name           = MACHINE_TYPE_NAME("raspi0"),
         .parent         = TYPE_RASPI_MACHINE,
@@ -389,6 +392,7 @@ static const TypeInfo raspi_machine_types[] = {
         .parent         = TYPE_RASPI_MACHINE,
         .class_init     = raspi2b_machine_class_init,
     },
+#endif /* CONFIG_TCG */
 #ifdef TARGET_AARCH64
     {
         .name           = MACHINE_TYPE_NAME("raspi3ap"),
-- 
2.26.2

Re: [RFC PATCH 2/2] hw/arm/raspi: Restrict BCM2835 / BCM2836 SoC to TCG
Posted by Luc Michel 5 years ago
Hi Philippe,

On 16:14 Sun 31 Jan     , Philippe Mathieu-Daudé wrote:
> KVM requires the target cpu to be at least ARMv8 architecture
> (support on ARMv7 has been dropped in commit 82bf7ae84ce:
> "target/arm: Remove KVM support for 32-bit Arm hosts").
Wow, is there absolutely no way to do that then? What about using an
ARMv8 and starting in AArch32 mode? Is that possible with KVM? I guess
it might not be strictly identical as spawning the "real" CPU...

> 
> From the various SoC used by the Raspberry Pi machines, only
> the BCM2837 is an ARMv8 (Cortex-A53).
> 
> Restrict the BCM2835 (ARM1176) and BCM2836 (Cortex-A7) to TCG.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/arm/bcm2836.c | 6 ++++++
>  hw/arm/raspi.c   | 4 ++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index fd16ed87c40..3051764f2dc 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -89,6 +89,7 @@ static bool bcm283x_common_realize(DeviceState *dev, Error **errp)
>      return true;
>  }
>  
> +#ifdef CONFIG_TCG
I'm not sure it's enough. TCG and KVM can be enabled in the same
binary. You'll have to perform a runtime check here I think.

>  static void bcm2835_realize(DeviceState *dev, Error **errp)
>  {
>      BCM283XState *s = BCM283X(dev);
> @@ -107,6 +108,7 @@ static void bcm2835_realize(DeviceState *dev, Error **errp)
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 1,
>              qdev_get_gpio_in(DEVICE(&s->cpu[0].core), ARM_CPU_FIQ));
>  }
> +#endif /* CONFIG_TCG */
>  
>  static void bcm2836_realize(DeviceState *dev, Error **errp)
>  {
> @@ -178,6 +180,7 @@ static void bcm283x_class_init(ObjectClass *oc, void *data)
>      dc->user_creatable = false;
>  }
>  
> +#ifdef CONFIG_TCG
>  static void bcm2835_class_init(ObjectClass *oc, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(oc);
> @@ -201,6 +204,7 @@ static void bcm2836_class_init(ObjectClass *oc, void *data)
>      bc->clusterid = 0xf;
>      dc->realize = bcm2836_realize;
>  };
> +#endif /* CONFIG_TCG */
>  
>  #ifdef TARGET_AARCH64
>  static void bcm2837_class_init(ObjectClass *oc, void *data)
> @@ -227,6 +231,7 @@ static const TypeInfo bcm283x_types[] = {
>          .class_init     = bcm283x_class_init,
>          .abstract       = true,
>      },
> +#ifdef CONFIG_TCG
>      {
>          .name           = TYPE_BCM2835,
>          .parent         = TYPE_BCM283X,
> @@ -236,6 +241,7 @@ static const TypeInfo bcm283x_types[] = {
>          .parent         = TYPE_BCM283X,
>          .class_init     = bcm2836_class_init,
>      },
> +#endif /* CONFIG_TCG */
>  #ifdef TARGET_AARCH64
>      {
>          .name           = TYPE_BCM2837,
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index dce966a4dd8..cfa15504d9c 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -319,6 +319,7 @@ static void raspi_machine_class_common_init(MachineClass *mc,
>      mc->default_ram_id = "ram";
>  };
>  
> +#ifdef CONFIG_TCG
>  static void raspi0_machine_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
> @@ -346,6 +347,7 @@ static void raspi2b_machine_class_init(ObjectClass *oc, void *data)
>      rmc->board_rev = 0xa21041;
>      raspi_machine_class_common_init(mc, rmc->board_rev);
>  };
> +#endif /* CONFIG_TCG */
>  
>  #ifdef TARGET_AARCH64
>  static void raspi3ap_machine_class_init(ObjectClass *oc, void *data)
> @@ -376,6 +378,7 @@ static const TypeInfo raspi_machine_types[] = {
>          .class_size     = sizeof(RaspiMachineClass),
>          .abstract       = true,
>      },
> +#ifdef CONFIG_TCG
>      {
>          .name           = MACHINE_TYPE_NAME("raspi0"),
>          .parent         = TYPE_RASPI_MACHINE,
> @@ -389,6 +392,7 @@ static const TypeInfo raspi_machine_types[] = {
>          .parent         = TYPE_RASPI_MACHINE,
>          .class_init     = raspi2b_machine_class_init,
>      },
> +#endif /* CONFIG_TCG */
>  #ifdef TARGET_AARCH64
>      {
>          .name           = MACHINE_TYPE_NAME("raspi3ap"),
> -- 
> 2.26.2
> 

-- 

Re: [RFC PATCH 2/2] hw/arm/raspi: Restrict BCM2835 / BCM2836 SoC to TCG
Posted by Peter Maydell 5 years ago
On Mon, 1 Feb 2021 at 08:18, Luc Michel <luc@lmichel.fr> wrote:
> On 16:14 Sun 31 Jan     , Philippe Mathieu-Daudé wrote:
> > KVM requires the target cpu to be at least ARMv8 architecture
> > (support on ARMv7 has been dropped in commit 82bf7ae84ce:
> > "target/arm: Remove KVM support for 32-bit Arm hosts").
> Wow, is there absolutely no way to do that then? What about using an
> ARMv8 and starting in AArch32 mode? Is that possible with KVM? I guess
> it might not be strictly identical as spawning the "real" CPU...

"Support hardware-accelerated emulation of older v7 CPUs" is
not a design goal of the virtualization extensions; you can't
do it. KVM does support having a guest CPU which is AArch32 for EL1,
but that will never be a v7 CPU, because it will be the same as
the host CPU, which will always be v8.

In general I would prefer that we don't try to do stuff to
make KVM kinda-sorta-work on random 32-bit boards by stuffing
in a not-the-right-type CPU, because this increases our
security boundary massively. At the moment we can reasonably
say "only the 'virt' board and one of the Xilinx boards are
security-critical".

thanks
-- PMM

Re: [RFC PATCH 2/2] hw/arm/raspi: Restrict BCM2835 / BCM2836 SoC to TCG
Posted by Philippe Mathieu-Daudé 5 years ago
On 2/2/21 1:28 PM, Peter Maydell wrote:
> On Mon, 1 Feb 2021 at 08:18, Luc Michel <luc@lmichel.fr> wrote:
>> On 16:14 Sun 31 Jan     , Philippe Mathieu-Daudé wrote:
>>> KVM requires the target cpu to be at least ARMv8 architecture
>>> (support on ARMv7 has been dropped in commit 82bf7ae84ce:
>>> "target/arm: Remove KVM support for 32-bit Arm hosts").
>> Wow, is there absolutely no way to do that then? What about using an
>> ARMv8 and starting in AArch32 mode? Is that possible with KVM? I guess
>> it might not be strictly identical as spawning the "real" CPU...
> 
> "Support hardware-accelerated emulation of older v7 CPUs" is
> not a design goal of the virtualization extensions; you can't
> do it. KVM does support having a guest CPU which is AArch32 for EL1,
> but that will never be a v7 CPU, because it will be the same as
> the host CPU, which will always be v8.
> 
> In general I would prefer that we don't try to do stuff to
> make KVM kinda-sorta-work on random 32-bit boards by stuffing
> in a not-the-right-type CPU, because this increases our
> security boundary massively.

Fine, as this simplifies many things.

> At the moment we can reasonably
> say "only the 'virt' board and one of the Xilinx boards are
> security-critical".

What about the SBSA-ref?

Thanks,

Phil.

Re: [RFC PATCH 2/2] hw/arm/raspi: Restrict BCM2835 / BCM2836 SoC to TCG
Posted by Peter Maydell 5 years ago
On Tue, 2 Feb 2021 at 13:29, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 2/2/21 1:28 PM, Peter Maydell wrote:
> > At the moment we can reasonably
> > say "only the 'virt' board and one of the Xilinx boards are
> > security-critical".
>
> What about the SBSA-ref?

It doesn't work with KVM, and enforces it:

    if (kvm_enabled()) {
        error_report("sbsa-ref: KVM is not supported for this machine");
        exit(1);
    }

thanks
-- PMM

Re: [RFC PATCH 2/2] hw/arm/raspi: Restrict BCM2835 / BCM2836 SoC to TCG
Posted by Philippe Mathieu-Daudé 5 years ago
On 2/2/21 2:47 PM, Peter Maydell wrote:
> On Tue, 2 Feb 2021 at 13:29, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> On 2/2/21 1:28 PM, Peter Maydell wrote:
>>> At the moment we can reasonably
>>> say "only the 'virt' board and one of the Xilinx boards are
>>> security-critical".
>>
>> What about the SBSA-ref?
> 
> It doesn't work with KVM, and enforces it:
> 
>     if (kvm_enabled()) {
>         error_report("sbsa-ref: KVM is not supported for this machine");
>         exit(1);
>     }

Uh I didn't know... That simplifies even further the KVM-only
build, thanks :)

Re: [RFC PATCH 2/2] hw/arm/raspi: Restrict BCM2835 / BCM2836 SoC to TCG
Posted by Philippe Mathieu-Daudé 5 years ago
+Igor (qom) / Eduardo (qdev) / Paolo (accel)

On 2/1/21 9:18 AM, Luc Michel wrote:
> Hi Philippe,
> 
> On 16:14 Sun 31 Jan     , Philippe Mathieu-Daudé wrote:
>> KVM requires the target cpu to be at least ARMv8 architecture
>> (support on ARMv7 has been dropped in commit 82bf7ae84ce:
>> "target/arm: Remove KVM support for 32-bit Arm hosts").
> Wow, is there absolutely no way to do that then? What about using an
> ARMv8 and starting in AArch32 mode? Is that possible with KVM? I guess
> it might not be strictly identical as spawning the "real" CPU...

This is what Peter said here:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg777669.html

  "KVM requires the target CPU to be at least ARMv8, because
  we only support the "host" cpu type, and all KVM host CPUs
  are v8, which means you can't pass a v7 CPU as the target CPU.
  (This used to not be true when we still supported running
  KVM on a v7 CPU like the Cortex-A15, in which case you could
  pass it to the guest.)"

> 
>>
>> From the various SoC used by the Raspberry Pi machines, only
>> the BCM2837 is an ARMv8 (Cortex-A53).
>>
>> Restrict the BCM2835 (ARM1176) and BCM2836 (Cortex-A7) to TCG.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/arm/bcm2836.c | 6 ++++++
>>  hw/arm/raspi.c   | 4 ++++
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
>> index fd16ed87c40..3051764f2dc 100644
>> --- a/hw/arm/bcm2836.c
>> +++ b/hw/arm/bcm2836.c
>> @@ -89,6 +89,7 @@ static bool bcm283x_common_realize(DeviceState *dev, Error **errp)
>>      return true;
>>  }
>>  
>> +#ifdef CONFIG_TCG
> I'm not sure it's enough. TCG and KVM can be enabled in the same
> binary. You'll have to perform a runtime check here I think.

If TCG is enabled, all SoC are built in (regardless of KVM enabled).
If only KVM is enabled, the TCG part is not built in (only ARMv8
based SoC left).

The problem is when QOM types are registered, we can not know
yet if the accelerator is enabled, because accelerators are also
QOM types and are realized later. So at this point runtime check
is not possible. See this post:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg777761.html

> 
>>  static void bcm2835_realize(DeviceState *dev, Error **errp)
>>  {
>>      BCM283XState *s = BCM283X(dev);
>> @@ -107,6 +108,7 @@ static void bcm2835_realize(DeviceState *dev, Error **errp)
>>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 1,
>>              qdev_get_gpio_in(DEVICE(&s->cpu[0].core), ARM_CPU_FIQ));
>>  }
>> +#endif /* CONFIG_TCG */
...

Re: [RFC PATCH 2/2] hw/arm/raspi: Restrict BCM2835 / BCM2836 SoC to TCG
Posted by Paolo Bonzini 5 years ago
On 01/02/21 09:46, Philippe Mathieu-Daudé wrote:
>>> +#ifdef CONFIG_TCG
>> I'm not sure it's enough. TCG and KVM can be enabled in the same
>> binary. You'll have to perform a runtime check here I think.
> If TCG is enabled, all SoC are built in (regardless of KVM enabled).
> If only KVM is enabled, the TCG part is not built in (only ARMv8
> based SoC left).
> 
> The problem is when QOM types are registered, we can not know
> yet if the accelerator is enabled, because accelerators are also
> QOM types and are realized later. So at this point runtime check
> is not possible. See this post:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg777761.html
> 

The check should be done on the CPU type, not on the accelerator.

On top of that you could add a "depends on TCG" to the hw/arm/Kconfig 
file, but Luc is correct that it would be just a nice-to-have and not 
the real fix.

Paolo