[PATCH] arm64: rsi: Add automatic arm-cca-guest module loading

Jeremy Linton posted 1 patch 3 weeks, 5 days ago
arch/arm64/include/asm/rsi.h                    |  2 ++
arch/arm64/kernel/rsi.c                         | 15 +++++++++++++++
drivers/virt/coco/arm-cca-guest/arm-cca-guest.c |  7 +++++++
3 files changed, 24 insertions(+)
[PATCH] arm64: rsi: Add automatic arm-cca-guest module loading
Posted by Jeremy Linton 3 weeks, 5 days ago
The TSM module provides both guest identification as well as
attestation when a guest is run in CCA mode. Lets assure by creating a
dummy platform device that the module is automatically loaded during
boot. Once it is in place it can be used earlier in the boot process
to say decrypt a LUKS rootfs.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 arch/arm64/include/asm/rsi.h                    |  2 ++
 arch/arm64/kernel/rsi.c                         | 15 +++++++++++++++
 drivers/virt/coco/arm-cca-guest/arm-cca-guest.c |  7 +++++++
 3 files changed, 24 insertions(+)

diff --git a/arch/arm64/include/asm/rsi.h b/arch/arm64/include/asm/rsi.h
index 188cbb9b23f5..1b14a4c4257a 100644
--- a/arch/arm64/include/asm/rsi.h
+++ b/arch/arm64/include/asm/rsi.h
@@ -10,6 +10,8 @@
 #include <linux/jump_label.h>
 #include <asm/rsi_cmds.h>
 
+#define ARMV9_RSI_PDEV_NAME "arm-cca-dev"
+
 DECLARE_STATIC_KEY_FALSE(rsi_present);
 
 void __init arm64_rsi_init(void);
diff --git a/arch/arm64/kernel/rsi.c b/arch/arm64/kernel/rsi.c
index 3031f25c32ef..ad963eb12921 100644
--- a/arch/arm64/kernel/rsi.c
+++ b/arch/arm64/kernel/rsi.c
@@ -8,6 +8,7 @@
 #include <linux/psci.h>
 #include <linux/swiotlb.h>
 #include <linux/cc_platform.h>
+#include <linux/platform_device.h>
 
 #include <asm/io.h>
 #include <asm/mem_encrypt.h>
@@ -140,3 +141,17 @@ void __init arm64_rsi_init(void)
 	static_branch_enable(&rsi_present);
 }
 
+static struct platform_device rsi_dev = {
+	.name = ARMV9_RSI_PDEV_NAME,
+	.id = -1
+};
+
+static int __init rsi_init(void)
+{
+	if (is_realm_world())
+		if (platform_device_register(&rsi_dev))
+			pr_err("failed to register rsi platform device");
+	return 0;
+}
+
+arch_initcall(rsi_init)
diff --git a/drivers/virt/coco/arm-cca-guest/arm-cca-guest.c b/drivers/virt/coco/arm-cca-guest/arm-cca-guest.c
index 488153879ec9..e7ef3b83d5d9 100644
--- a/drivers/virt/coco/arm-cca-guest/arm-cca-guest.c
+++ b/drivers/virt/coco/arm-cca-guest/arm-cca-guest.c
@@ -6,6 +6,7 @@
 #include <linux/arm-smccc.h>
 #include <linux/cc_platform.h>
 #include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/smp.h>
 #include <linux/tsm.h>
@@ -219,6 +220,12 @@ static void __exit arm_cca_guest_exit(void)
 }
 module_exit(arm_cca_guest_exit);
 
+static const struct platform_device_id arm_cca_match[] = {
+	{ ARMV9_RSI_PDEV_NAME, 0},
+	{ }
+};
+
+MODULE_DEVICE_TABLE(platform, arm_cca_match);
 MODULE_AUTHOR("Sami Mujawar <sami.mujawar@arm.com>");
 MODULE_DESCRIPTION("Arm CCA Guest TSM Driver");
 MODULE_LICENSE("GPL");
-- 
2.46.0
Re: [PATCH] arm64: rsi: Add automatic arm-cca-guest module loading
Posted by kernel test robot 3 weeks, 2 days ago
Hi Jeremy,

kernel test robot noticed the following build warnings:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on next-20241101]
[cannot apply to kvmarm/next soc/for-next linus/master arm/for-next arm/fixes v6.12-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jeremy-Linton/arm64-rsi-Add-automatic-arm-cca-guest-module-loading/20241029-221213
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
patch link:    https://lore.kernel.org/r/20241029141114.7207-1-jeremy.linton%40arm.com
patch subject: [PATCH] arm64: rsi: Add automatic arm-cca-guest module loading
config: arm64-randconfig-002-20241101 (https://download.01.org/0day-ci/archive/20241102/202411020102.mnsnqAiD-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 639a7ac648f1e50ccd2556e17d401c04f9cce625)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241102/202411020102.mnsnqAiD-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411020102.mnsnqAiD-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/virt/coco/arm-cca-guest/arm-cca-guest.c:223:40: warning: unused variable 'arm_cca_match' [-Wunused-const-variable]
     223 | static const struct platform_device_id arm_cca_match[] = {
         |                                        ^~~~~~~~~~~~~
   1 warning generated.


vim +/arm_cca_match +223 drivers/virt/coco/arm-cca-guest/arm-cca-guest.c

   222	
 > 223	static const struct platform_device_id arm_cca_match[] = {
   224		{ ARMV9_RSI_PDEV_NAME, 0},
   225		{ }
   226	};
   227	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] arm64: rsi: Add automatic arm-cca-guest module loading
Posted by kernel test robot 3 weeks, 2 days ago
Hi Jeremy,

kernel test robot noticed the following build warnings:

[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on next-20241101]
[cannot apply to kvmarm/next soc/for-next linus/master arm/for-next arm/fixes v6.12-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jeremy-Linton/arm64-rsi-Add-automatic-arm-cca-guest-module-loading/20241029-221213
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
patch link:    https://lore.kernel.org/r/20241029141114.7207-1-jeremy.linton%40arm.com
patch subject: [PATCH] arm64: rsi: Add automatic arm-cca-guest module loading
config: arm64-randconfig-001-20241101 (https://download.01.org/0day-ci/archive/20241102/202411020124.OBmJyKCY-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241102/202411020124.OBmJyKCY-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411020124.OBmJyKCY-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/virt/coco/arm-cca-guest/arm-cca-guest.c:223:40: warning: 'arm_cca_match' defined but not used [-Wunused-const-variable=]
     223 | static const struct platform_device_id arm_cca_match[] = {
         |                                        ^~~~~~~~~~~~~


vim +/arm_cca_match +223 drivers/virt/coco/arm-cca-guest/arm-cca-guest.c

   222	
 > 223	static const struct platform_device_id arm_cca_match[] = {
   224		{ ARMV9_RSI_PDEV_NAME, 0},
   225		{ }
   226	};
   227	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] arm64: rsi: Add automatic arm-cca-guest module loading
Posted by Gavin Shan 3 weeks, 5 days ago
Hi Jeremy,

On 10/30/24 12:11 AM, Jeremy Linton wrote:
> The TSM module provides both guest identification as well as
> attestation when a guest is run in CCA mode. Lets assure by creating a
> dummy platform device that the module is automatically loaded during
> boot. Once it is in place it can be used earlier in the boot process
> to say decrypt a LUKS rootfs.
> 
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>   arch/arm64/include/asm/rsi.h                    |  2 ++
>   arch/arm64/kernel/rsi.c                         | 15 +++++++++++++++
>   drivers/virt/coco/arm-cca-guest/arm-cca-guest.c |  7 +++++++
>   3 files changed, 24 insertions(+)
> 

I don't understand how the TSM module is automatically loaded and arm_cca_guest_init()
is triggered because of the newly introduced platform device. Could you please provide
more details? Apart from it, some nick-picks as below.

> diff --git a/arch/arm64/include/asm/rsi.h b/arch/arm64/include/asm/rsi.h
> index 188cbb9b23f5..1b14a4c4257a 100644
> --- a/arch/arm64/include/asm/rsi.h
> +++ b/arch/arm64/include/asm/rsi.h
> @@ -10,6 +10,8 @@
>   #include <linux/jump_label.h>
>   #include <asm/rsi_cmds.h>
>   
> +#define ARMV9_RSI_PDEV_NAME "arm-cca-dev"
> +

Maybe 'ARMV9' can be avoided since RSI is the specific feature to ARMv9.
Besides, we already had @rsi_present there. So I would suggest to rename
it to RSI_PDEV_NAME

>   DECLARE_STATIC_KEY_FALSE(rsi_present);
>   
>   void __init arm64_rsi_init(void);
> diff --git a/arch/arm64/kernel/rsi.c b/arch/arm64/kernel/rsi.c
> index 3031f25c32ef..ad963eb12921 100644
> --- a/arch/arm64/kernel/rsi.c
> +++ b/arch/arm64/kernel/rsi.c
> @@ -8,6 +8,7 @@
>   #include <linux/psci.h>
>   #include <linux/swiotlb.h>
>   #include <linux/cc_platform.h>
> +#include <linux/platform_device.h>
>   
>   #include <asm/io.h>
>   #include <asm/mem_encrypt.h>
> @@ -140,3 +141,17 @@ void __init arm64_rsi_init(void)
>   	static_branch_enable(&rsi_present);
>   }
>   
> +static struct platform_device rsi_dev = {
> +	.name = ARMV9_RSI_PDEV_NAME,
> +	.id = -1
> +};
> +

         .id = PLATFORM_DEVID_NONE,

> +static int __init rsi_init(void)
> +{
> +	if (is_realm_world())
> +		if (platform_device_register(&rsi_dev))
> +			pr_err("failed to register rsi platform device");
> +	return 0;
> +}
> +

Those two checks can be connected with '&&' and '\n' seems missed in the
error message.

         if (is_realm_world() && platform_device_register(&rsi_dev))
             pr_err("Failed to register RSI platform device\n");

> +arch_initcall(rsi_init)
> diff --git a/drivers/virt/coco/arm-cca-guest/arm-cca-guest.c b/drivers/virt/coco/arm-cca-guest/arm-cca-guest.c
> index 488153879ec9..e7ef3b83d5d9 100644
> --- a/drivers/virt/coco/arm-cca-guest/arm-cca-guest.c
> +++ b/drivers/virt/coco/arm-cca-guest/arm-cca-guest.c
> @@ -6,6 +6,7 @@
>   #include <linux/arm-smccc.h>
>   #include <linux/cc_platform.h>
>   #include <linux/kernel.h>
> +#include <linux/mod_devicetable.h>
>   #include <linux/module.h>
>   #include <linux/smp.h>
>   #include <linux/tsm.h>
> @@ -219,6 +220,12 @@ static void __exit arm_cca_guest_exit(void)
>   }
>   module_exit(arm_cca_guest_exit);
>   
> +static const struct platform_device_id arm_cca_match[] = {
> +	{ ARMV9_RSI_PDEV_NAME, 0},
> +	{ }
> +};
> +
> +MODULE_DEVICE_TABLE(platform, arm_cca_match);


/* Comments here to explain why @arm_cca_dev_ids[] is needed */
static const struct platform_device_id arm_cca_dev_ids[] = {
        ...
};

MODULE_DEVICE_TABLE(platform, arm_cca_dev_ids);

>   MODULE_AUTHOR("Sami Mujawar <sami.mujawar@arm.com>");
>   MODULE_DESCRIPTION("Arm CCA Guest TSM Driver");
>   MODULE_LICENSE("GPL");

Thanks,
Gavin
Re: [PATCH] arm64: rsi: Add automatic arm-cca-guest module loading
Posted by Jeremy Linton 3 weeks, 4 days ago
Hi Gavin,

Thanks for looking at this!

On 10/29/24 7:23 PM, Gavin Shan wrote:
> Hi Jeremy,
> 
> On 10/30/24 12:11 AM, Jeremy Linton wrote:
>> The TSM module provides both guest identification as well as
>> attestation when a guest is run in CCA mode. Lets assure by creating a
>> dummy platform device that the module is automatically loaded during
>> boot. Once it is in place it can be used earlier in the boot process
>> to say decrypt a LUKS rootfs.
>>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>>   arch/arm64/include/asm/rsi.h                    |  2 ++
>>   arch/arm64/kernel/rsi.c                         | 15 +++++++++++++++
>>   drivers/virt/coco/arm-cca-guest/arm-cca-guest.c |  7 +++++++
>>   3 files changed, 24 insertions(+)
>>
> 
> I don't understand how the TSM module is automatically loaded and 
> arm_cca_guest_init()
> is triggered because of the newly introduced platform device. Could you 
> please provide
> more details? Apart from it, some nick-picks as below.

I think your asking how the module boilerplate here works, AKA how the 
standard uevent/udev/modalias/kmod stuff works? The short version is 
that the platform bus uevents an add device with a modalias and 
userspace udev + kmod finds matching modules, and their dependencies, 
and loads them which triggers the module_init() calls.

The suse folks have a detailed description of how this works:
https://doc.opensuse.org/documentation/leap/reference/html/book-reference/cha-udev.html#sec-udev-kernel

So, this is a fairly common misuse of the platform bus, in this case to 
avoid needing a HWCAP. Assuring the module exists in the initrd will 
then result in it being loaded along any other modules required for the 
rootfs pivot.


> 
>> diff --git a/arch/arm64/include/asm/rsi.h b/arch/arm64/include/asm/rsi.h
>> index 188cbb9b23f5..1b14a4c4257a 100644
>> --- a/arch/arm64/include/asm/rsi.h
>> +++ b/arch/arm64/include/asm/rsi.h
>> @@ -10,6 +10,8 @@
>>   #include <linux/jump_label.h>
>>   #include <asm/rsi_cmds.h>
>> +#define ARMV9_RSI_PDEV_NAME "arm-cca-dev"
>> +
> 
> Maybe 'ARMV9' can be avoided since RSI is the specific feature to ARMv9.
> Besides, we already had @rsi_present there. So I would suggest to rename
> it to RSI_PDEV_NAME

This and the remainder of the comments below look reasonable to me, thanks!
> 
>>   DECLARE_STATIC_KEY_FALSE(rsi_present);
>>   void __init arm64_rsi_init(void);
>> diff --git a/arch/arm64/kernel/rsi.c b/arch/arm64/kernel/rsi.c
>> index 3031f25c32ef..ad963eb12921 100644
>> --- a/arch/arm64/kernel/rsi.c
>> +++ b/arch/arm64/kernel/rsi.c
>> @@ -8,6 +8,7 @@
>>   #include <linux/psci.h>
>>   #include <linux/swiotlb.h>
>>   #include <linux/cc_platform.h>
>> +#include <linux/platform_device.h>
>>   #include <asm/io.h>
>>   #include <asm/mem_encrypt.h>
>> @@ -140,3 +141,17 @@ void __init arm64_rsi_init(void)
>>       static_branch_enable(&rsi_present);
>>   }
>> +static struct platform_device rsi_dev = {
>> +    .name = ARMV9_RSI_PDEV_NAME,
>> +    .id = -1
>> +};
>> +
> 
>          .id = PLATFORM_DEVID_NONE,
> 
>> +static int __init rsi_init(void)
>> +{
>> +    if (is_realm_world())
>> +        if (platform_device_register(&rsi_dev))
>> +            pr_err("failed to register rsi platform device");
>> +    return 0;
>> +}
>> +
> 
> Those two checks can be connected with '&&' and '\n' seems missed in the
> error message.
> 
>          if (is_realm_world() && platform_device_register(&rsi_dev))
>              pr_err("Failed to register RSI platform device\n");
> 
>> +arch_initcall(rsi_init)
>> diff --git a/drivers/virt/coco/arm-cca-guest/arm-cca-guest.c b/ 
>> drivers/virt/coco/arm-cca-guest/arm-cca-guest.c
>> index 488153879ec9..e7ef3b83d5d9 100644
>> --- a/drivers/virt/coco/arm-cca-guest/arm-cca-guest.c
>> +++ b/drivers/virt/coco/arm-cca-guest/arm-cca-guest.c
>> @@ -6,6 +6,7 @@
>>   #include <linux/arm-smccc.h>
>>   #include <linux/cc_platform.h>
>>   #include <linux/kernel.h>
>> +#include <linux/mod_devicetable.h>
>>   #include <linux/module.h>
>>   #include <linux/smp.h>
>>   #include <linux/tsm.h>
>> @@ -219,6 +220,12 @@ static void __exit arm_cca_guest_exit(void)
>>   }
>>   module_exit(arm_cca_guest_exit);
>> +static const struct platform_device_id arm_cca_match[] = {
>> +    { ARMV9_RSI_PDEV_NAME, 0},
>> +    { }
>> +};
>> +
>> +MODULE_DEVICE_TABLE(platform, arm_cca_match);
> 
> 
> /* Comments here to explain why @arm_cca_dev_ids[] is needed */
> static const struct platform_device_id arm_cca_dev_ids[] = {
>         ...
> };
> 
> MODULE_DEVICE_TABLE(platform, arm_cca_dev_ids);
> 
>>   MODULE_AUTHOR("Sami Mujawar <sami.mujawar@arm.com>");
>>   MODULE_DESCRIPTION("Arm CCA Guest TSM Driver");
>>   MODULE_LICENSE("GPL");
> 
> Thanks,
> Gavin
> 

Re: [PATCH] arm64: rsi: Add automatic arm-cca-guest module loading
Posted by Gavin Shan 3 weeks, 4 days ago
Hi Jeremy,

On 10/31/24 1:16 AM, Jeremy Linton wrote:
> On 10/29/24 7:23 PM, Gavin Shan wrote:
>> On 10/30/24 12:11 AM, Jeremy Linton wrote:
>>> The TSM module provides both guest identification as well as
>>> attestation when a guest is run in CCA mode. Lets assure by creating a
>>> dummy platform device that the module is automatically loaded during
>>> boot. Once it is in place it can be used earlier in the boot process
>>> to say decrypt a LUKS rootfs.
>>>
>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>> ---
>>>   arch/arm64/include/asm/rsi.h                    |  2 ++
>>>   arch/arm64/kernel/rsi.c                         | 15 +++++++++++++++
>>>   drivers/virt/coco/arm-cca-guest/arm-cca-guest.c |  7 +++++++
>>>   3 files changed, 24 insertions(+)
>>>
>>
>> I don't understand how the TSM module is automatically loaded and arm_cca_guest_init()
>> is triggered because of the newly introduced platform device. Could you please provide
>> more details? Apart from it, some nick-picks as below.
> 
> I think your asking how the module boilerplate here works, AKA how the standard uevent/udev/modalias/kmod stuff works? The short version is that the platform bus uevents an add device with a modalias and userspace udev + kmod finds matching modules, and their dependencies, and loads them which triggers the module_init() calls.
> 
> The suse folks have a detailed description of how this works:
> https://doc.opensuse.org/documentation/leap/reference/html/book-reference/cha-udev.html#sec-udev-kernel
> 
> So, this is a fairly common misuse of the platform bus, in this case to avoid needing a HWCAP. Assuring the module exists in the initrd will then result in it being loaded along any other modules required for the rootfs pivot.
> 
> 

Thanks for the explanation and details. The module won't be automatically loaded if
udev daemon isn't in place or the DEV_ADD event is ignored for whatever reasons. For
example the corresponding ACTION for DEV_ADD of this particular device is null in the
udev rules. So it's not guranteed that the module can be automatically loaded until udev
is in place and udev rules have been configured properly. It's a best-effort attempt
if I don't miss anything.

Could you please update the change log to mention the automatic module loading depends
on udev and its rules? In this way, readers will know it's a best-effort attempt at least.

Thanks,
Gavin

Re: [PATCH] arm64: rsi: Add automatic arm-cca-guest module loading
Posted by Jeremy Linton 3 weeks, 4 days ago
Hi,

On 10/30/24 5:48 PM, Gavin Shan wrote:
> Hi Jeremy,
> 
> On 10/31/24 1:16 AM, Jeremy Linton wrote:
>> On 10/29/24 7:23 PM, Gavin Shan wrote:
>>> On 10/30/24 12:11 AM, Jeremy Linton wrote:
>>>> The TSM module provides both guest identification as well as
>>>> attestation when a guest is run in CCA mode. Lets assure by creating a
>>>> dummy platform device that the module is automatically loaded during
>>>> boot. Once it is in place it can be used earlier in the boot process
>>>> to say decrypt a LUKS rootfs.
>>>>
>>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>>> ---
>>>>   arch/arm64/include/asm/rsi.h                    |  2 ++
>>>>   arch/arm64/kernel/rsi.c                         | 15 +++++++++++++++
>>>>   drivers/virt/coco/arm-cca-guest/arm-cca-guest.c |  7 +++++++
>>>>   3 files changed, 24 insertions(+)
>>>>
>>>
>>> I don't understand how the TSM module is automatically loaded and 
>>> arm_cca_guest_init()
>>> is triggered because of the newly introduced platform device. Could 
>>> you please provide
>>> more details? Apart from it, some nick-picks as below.
>>
>> I think your asking how the module boilerplate here works, AKA how the 
>> standard uevent/udev/modalias/kmod stuff works? The short version is 
>> that the platform bus uevents an add device with a modalias and 
>> userspace udev + kmod finds matching modules, and their dependencies, 
>> and loads them which triggers the module_init() calls.
>>
>> The suse folks have a detailed description of how this works:
>> https://doc.opensuse.org/documentation/leap/reference/html/book- 
>> reference/cha-udev.html#sec-udev-kernel
>>
>> So, this is a fairly common misuse of the platform bus, in this case 
>> to avoid needing a HWCAP. Assuring the module exists in the initrd 
>> will then result in it being loaded along any other modules required 
>> for the rootfs pivot.
>>
>>
> 
> Thanks for the explanation and details. The module won't be 
> automatically loaded if
> udev daemon isn't in place or the DEV_ADD event is ignored for whatever 
> reasons. For
> example the corresponding ACTION for DEV_ADD of this particular device 
> is null in the
> udev rules. So it's not guranteed that the module can be automatically 
> loaded until udev
> is in place and udev rules have been configured properly. It's a best- 
> effort attempt
> if I don't miss anything.

This functionality has been standard in all but the most deeply 
enmbedded linux systems for a couple decades now (AFAIK). The platform 
and modalias logic should largely just work everywhere that its 
appropriate to be building this as a module. And to be clear that is 
without updating any of the existing rules.

> 
> Could you please update the change log to mention the automatic module 
> loading depends
> on udev and its rules? In this way, readers will know it's a best-effort 
> attempt at least.
> 
> Thanks,
> Gavin
> 

Re: [PATCH] arm64: rsi: Add automatic arm-cca-guest module loading
Posted by Gavin Shan 3 weeks, 3 days ago
Hi Jeremy

On 10/31/24 12:08 PM, Jeremy Linton wrote:
> On 10/30/24 5:48 PM, Gavin Shan wrote:
>> On 10/31/24 1:16 AM, Jeremy Linton wrote:
>>> On 10/29/24 7:23 PM, Gavin Shan wrote:
>>>> On 10/30/24 12:11 AM, Jeremy Linton wrote:
>>>>> The TSM module provides both guest identification as well as
>>>>> attestation when a guest is run in CCA mode. Lets assure by creating a
>>>>> dummy platform device that the module is automatically loaded during
>>>>> boot. Once it is in place it can be used earlier in the boot process
>>>>> to say decrypt a LUKS rootfs.
>>>>>
>>>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>>>> ---
>>>>>   arch/arm64/include/asm/rsi.h                    |  2 ++
>>>>>   arch/arm64/kernel/rsi.c                         | 15 +++++++++++++++
>>>>>   drivers/virt/coco/arm-cca-guest/arm-cca-guest.c |  7 +++++++
>>>>>   3 files changed, 24 insertions(+)
>>>>>
>>>>
>>>> I don't understand how the TSM module is automatically loaded and arm_cca_guest_init()
>>>> is triggered because of the newly introduced platform device. Could you please provide
>>>> more details? Apart from it, some nick-picks as below.
>>>
>>> I think your asking how the module boilerplate here works, AKA how the standard uevent/udev/modalias/kmod stuff works? The short version is that the platform bus uevents an add device with a modalias and userspace udev + kmod finds matching modules, and their dependencies, and loads them which triggers the module_init() calls.
>>>
>>> The suse folks have a detailed description of how this works:
>>> https://doc.opensuse.org/documentation/leap/reference/html/book- reference/cha-udev.html#sec-udev-kernel
>>>
>>> So, this is a fairly common misuse of the platform bus, in this case to avoid needing a HWCAP. Assuring the module exists in the initrd will then result in it being loaded along any other modules required for the rootfs pivot.
>>>
>>>
>>
>> Thanks for the explanation and details. The module won't be automatically loaded if
>> udev daemon isn't in place or the DEV_ADD event is ignored for whatever reasons. For
>> example the corresponding ACTION for DEV_ADD of this particular device is null in the
>> udev rules. So it's not guranteed that the module can be automatically loaded until udev
>> is in place and udev rules have been configured properly. It's a best- effort attempt
>> if I don't miss anything.
> 
> This functionality has been standard in all but the most deeply enmbedded linux systems for a couple decades now (AFAIK). The platform and modalias logic should largely just work everywhere that its appropriate to be building this as a module. And to be clear that is without updating any of the existing rules.
> 

Right, it's also what I understood. What I requested is just to mention it
in the change log if you agree, something like below. With this, the change
log looks complete to me.

"The TSM module will be loaded by udev daemon after it receives the device addition event."

>>
>> Could you please update the change log to mention the automatic module loading depends
>> on udev and its rules? In this way, readers will know it's a best-effort attempt at least.
>>

Thanks,
Gavin