Re: [PATCH v5 03/22] cc_platform: Add new attribute to prevent ACPI memory hotplug

[Igor Mammedov]

On Wed, 22 Jun 2022 13:45:01 +0200
"Rafael J. Wysocki" <rafael@xxxxxxxxxx> wrote:

> On Wed, Jun 22, 2022 at 1:16 PM Kai Huang <kai.huang@xxxxxxxxx> wrote:
> >
> > Platforms with confidential computing technology may not support ACPI
> > memory hotplug when such technology is enabled by the BIOS.  Examples
> > include Intel platforms which support Intel Trust Domain Extensions
> > (TDX).
> >
> > If the kernel ever receives ACPI memory hotplug event, it is likely a
> > BIOS bug.  For ACPI memory hot-add, the kernel should speak out this is
> > a BIOS bug and reject the new memory.  For hot-removal, for simplicity
> > just assume the kernel cannot continue to work normally, and just BUG().
> >
> > Add a new attribute CC_ATTR_ACPI_MEMORY_HOTPLUG_DISABLED to indicate the
> > platform doesn't support ACPI memory hotplug, so that kernel can handle
> > ACPI memory hotplug events for such platform.
> >
> > In acpi_memory_device_{add|remove}(), add early check against this
> > attribute and handle accordingly if it is set.
> >
> > Signed-off-by: Kai Huang <kai.huang@xxxxxxxxx>
> > ---
> >  drivers/acpi/acpi_memhotplug.c | 23 +++++++++++++++++++++++
> >  include/linux/cc_platform.h    | 10 ++++++++++
> >  2 files changed, 33 insertions(+)
> >
> > diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
> > index 24f662d8bd39..94d6354ea453 100644
> > --- a/drivers/acpi/acpi_memhotplug.c
> > +++ b/drivers/acpi/acpi_memhotplug.c
> > @@ -15,6 +15,7 @@
> >  #include <linux/acpi.h>
> >  #include <linux/memory.h>
> >  #include <linux/memory_hotplug.h>
> > +#include <linux/cc_platform.h>
> >
> >  #include "internal.h"
> >
> > @@ -291,6 +292,17 @@ static int acpi_memory_device_add(struct acpi_device *device,
> >         if (!device)
> >                 return -EINVAL;
> >
> > +       /*
> > +        * If the confidential computing platform doesn't support ACPI
> > +        * memory hotplug, the BIOS should never deliver such event to
> > +        * the kernel.  Report ACPI CPU hot-add as a BIOS bug and ignore
> > +        * the memory device.
> > +        */
> > +       if (cc_platform_has(CC_ATTR_ACPI_MEMORY_HOTPLUG_DISABLED)) {  
> 
> Same comment as for the acpi_processor driver: this will affect the
> initialization too and it would be cleaner to reset the
> .hotplug.enabled flag of the scan handler.

with QEMU, it is likely broken when memory is added as
  '-device pc-dimm'
on CLI since it's advertised only as device node in DSDT.

> 
> > +               dev_err(&device->dev, "[BIOS bug]: Platform doesn't support ACPI memory hotplug. New memory device ignored.\n");
> > +               return -EINVAL;
> > +       }
> > +
> >         mem_device = kzalloc(sizeof(struct acpi_memory_device), GFP_KERNEL);
> >         if (!mem_device)
> >                 return -ENOMEM;
> > @@ -334,6 +346,17 @@ static void acpi_memory_device_remove(struct acpi_device *device)
> >         if (!device || !acpi_driver_data(device))
> >                 return;
> >
> > +       /*
> > +        * The confidential computing platform is broken if ACPI memory
> > +        * hot-removal isn't supported but it happened anyway.  Assume
> > +        * it is not guaranteed that the kernel can continue to work
> > +        * normally.  Just BUG().
> > +        */
> > +       if (cc_platform_has(CC_ATTR_ACPI_CPU_HOTPLUG_DISABLED)) {
> > +               dev_err(&device->dev, "Platform doesn't support ACPI memory hotplug. BUG().\n");
> > +               BUG();
> > +       }
> > +
> >         mem_device = acpi_driver_data(device);
> >         acpi_memory_remove_memory(mem_device);
> >         acpi_memory_device_free(mem_device);
> > diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h
> > index 9ce9256facc8..b831c24bd7f6 100644
> > --- a/include/linux/cc_platform.h
> > +++ b/include/linux/cc_platform.h
> > @@ -93,6 +93,16 @@ enum cc_attr {
> >          * Examples include TDX platform.
> >          */
> >         CC_ATTR_ACPI_CPU_HOTPLUG_DISABLED,
> > +
> > +       /**
> > +        * @CC_ATTR_ACPI_MEMORY_HOTPLUG_DISABLED: ACPI memory hotplug is
> > +        *                                        not supported.
> > +        *
> > +        * The platform/os does not support ACPI memory hotplug.
> > +        *
> > +        * Examples include TDX platform.
> > +        */
> > +       CC_ATTR_ACPI_MEMORY_HOTPLUG_DISABLED,
> >  };
> >
> >  #ifdef CONFIG_ARCH_HAS_CC_PLATFORM
> > --
> > 2.36.1
> >  
>