diff mbox series

[RFC,07/14] efi_loader: capsule: add memory range capsule definitions

Message ID 20200317021247.5849-8-takahiro.akashi@linaro.org
State RFC
Delegated to: Heinrich Schuchardt
Headers show
Series efi_loader: add capsule update support | expand

Commit Message

Takahiro Akashi March 17, 2020, 2:12 a.m. UTC
Memory range capsule gives us a way to notify that some memory regions
should be left untouched across the next reset.
See UEFI specification, section 8.5.3.

Since how we should handle this kind of capsule is totally up to
the system, no implementation will be added in this commit.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 include/efi_api.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Heinrich Schuchardt March 17, 2020, 8:11 a.m. UTC | #1
On 3/17/20 3:12 AM, AKASHI Takahiro wrote:
> Memory range capsule gives us a way to notify that some memory regions
> should be left untouched across the next reset.
> See UEFI specification, section 8.5.3.

There are multiple places where RAM availability is tested by writing
during board bring up. Please, describe which changes are needed in ATF
and U-Boot to make memory range capsules work reliably.

Consider also memory encryption provided by some CPUs.
https://www.tomshardware.com/news/intel-mktme-amd-memory-encryption,39467.html

Aren't file based capsules a much more reliable way for the OS to
communicate to the firmware?

Best regards

Heinrich

>
> Since how we should handle this kind of capsule is totally up to
> the system, no implementation will be added in this commit.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   include/efi_api.h | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
>
> diff --git a/include/efi_api.h b/include/efi_api.h
> index ac2b38801c0c..b7bf21cac7ad 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -221,6 +221,10 @@ enum efi_reset_type {
>   	EFI_GUID(0x39b68c46, 0xf7fb, 0x441b, 0xb6, 0xec, \
>   		 0x16, 0xb0, 0xf6, 0x98, 0x21, 0xf3)
>
> +#define EFI_MEMORY_RANGE_CAPSULE_GUID \
> +	EFI_GUID(0xde9f0ec, 0x88b6, 0x428f, 0x97, 0x7a, \
> +		 0x25, 0x8f, 0x1d, 0xe, 0x5e, 0x72)
> +
>   struct efi_capsule_header {
>   	efi_guid_t capsule_guid;
>   	u32 header_size;
> @@ -236,6 +240,19 @@ struct efi_capsule_result_variable_header {
>   	efi_status_t capsule_status;
>   } __packed;
>
> +struct efi_memory_range {
> +	efi_physical_addr_t	address;
> +	u64			length;
> +};
> +
> +struct efi_memory_range_capsule {
> +	struct efi_capsule_header *header;
> +	/* EFI_MEMORY_TYPE: 0x80000000-0xFFFFFFFF */
> +	enum efi_mem_type os_requested_memory_type;
> +	u64 number_of_memory_ranges;
> +	struct efi_memory_range memory_ranges[];
> +} __packed;
> +
>   #define EFI_RT_SUPPORTED_GET_TIME			0x0001
>   #define EFI_RT_SUPPORTED_SET_TIME			0x0002
>   #define EFI_RT_SUPPORTED_GET_WAKEUP_TIME		0x0004
>
Takahiro Akashi March 18, 2020, 1:22 a.m. UTC | #2
On Tue, Mar 17, 2020 at 09:11:43AM +0100, Heinrich Schuchardt wrote:
> On 3/17/20 3:12 AM, AKASHI Takahiro wrote:
> > Memory range capsule gives us a way to notify that some memory regions
> > should be left untouched across the next reset.
> > See UEFI specification, section 8.5.3.
> 
> There are multiple places where RAM availability is tested by writing
> during board bring up. Please, describe which changes are needed in ATF
> and U-Boot to make memory range capsules work reliably.

I think that they are quite system/platform-specific
and that it's beyond the scope of this patch.

Thanks,
-Takahiro Akashi


> Consider also memory encryption provided by some CPUs.
> https://www.tomshardware.com/news/intel-mktme-amd-memory-encryption,39467.html
> 
> Aren't file based capsules a much more reliable way for the OS to
> communicate to the firmware?
> 
> Best regards
> 
> Heinrich
> 
> > 
> > Since how we should handle this kind of capsule is totally up to
> > the system, no implementation will be added in this commit.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >   include/efi_api.h | 17 +++++++++++++++++
> >   1 file changed, 17 insertions(+)
> > 
> > diff --git a/include/efi_api.h b/include/efi_api.h
> > index ac2b38801c0c..b7bf21cac7ad 100644
> > --- a/include/efi_api.h
> > +++ b/include/efi_api.h
> > @@ -221,6 +221,10 @@ enum efi_reset_type {
> >   	EFI_GUID(0x39b68c46, 0xf7fb, 0x441b, 0xb6, 0xec, \
> >   		 0x16, 0xb0, 0xf6, 0x98, 0x21, 0xf3)
> > 
> > +#define EFI_MEMORY_RANGE_CAPSULE_GUID \
> > +	EFI_GUID(0xde9f0ec, 0x88b6, 0x428f, 0x97, 0x7a, \
> > +		 0x25, 0x8f, 0x1d, 0xe, 0x5e, 0x72)
> > +
> >   struct efi_capsule_header {
> >   	efi_guid_t capsule_guid;
> >   	u32 header_size;
> > @@ -236,6 +240,19 @@ struct efi_capsule_result_variable_header {
> >   	efi_status_t capsule_status;
> >   } __packed;
> > 
> > +struct efi_memory_range {
> > +	efi_physical_addr_t	address;
> > +	u64			length;
> > +};
> > +
> > +struct efi_memory_range_capsule {
> > +	struct efi_capsule_header *header;
> > +	/* EFI_MEMORY_TYPE: 0x80000000-0xFFFFFFFF */
> > +	enum efi_mem_type os_requested_memory_type;
> > +	u64 number_of_memory_ranges;
> > +	struct efi_memory_range memory_ranges[];
> > +} __packed;
> > +
> >   #define EFI_RT_SUPPORTED_GET_TIME			0x0001
> >   #define EFI_RT_SUPPORTED_SET_TIME			0x0002
> >   #define EFI_RT_SUPPORTED_GET_WAKEUP_TIME		0x0004
> > 
>
Heinrich Schuchardt March 18, 2020, 7:35 a.m. UTC | #3
On 3/18/20 2:22 AM, AKASHI Takahiro wrote:
> On Tue, Mar 17, 2020 at 09:11:43AM +0100, Heinrich Schuchardt wrote:
>> On 3/17/20 3:12 AM, AKASHI Takahiro wrote:
>>> Memory range capsule gives us a way to notify that some memory regions
>>> should be left untouched across the next reset.
>>> See UEFI specification, section 8.5.3.
>>
>> There are multiple places where RAM availability is tested by writing
>> during board bring up. Please, describe which changes are needed in ATF
>> and U-Boot to make memory range capsules work reliably.
>
> I think that they are quite system/platform-specific
> and that it's beyond the scope of this patch.
>
> Thanks,
> -Takahiro Akashi

To me it does not make sense to merge this development if the platform
compatibility is not evaluated.

Best regards

Heinrich

>
>
>> Consider also memory encryption provided by some CPUs.
>> https://www.tomshardware.com/news/intel-mktme-amd-memory-encryption,39467.html
>>
>> Aren't file based capsules a much more reliable way for the OS to
>> communicate to the firmware?
>>
>> Best regards
>>
>> Heinrich
>>
>>>
>>> Since how we should handle this kind of capsule is totally up to
>>> the system, no implementation will be added in this commit.
>>>
>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>>> ---
>>>    include/efi_api.h | 17 +++++++++++++++++
>>>    1 file changed, 17 insertions(+)
>>>
>>> diff --git a/include/efi_api.h b/include/efi_api.h
>>> index ac2b38801c0c..b7bf21cac7ad 100644
>>> --- a/include/efi_api.h
>>> +++ b/include/efi_api.h
>>> @@ -221,6 +221,10 @@ enum efi_reset_type {
>>>    	EFI_GUID(0x39b68c46, 0xf7fb, 0x441b, 0xb6, 0xec, \
>>>    		 0x16, 0xb0, 0xf6, 0x98, 0x21, 0xf3)
>>>
>>> +#define EFI_MEMORY_RANGE_CAPSULE_GUID \
>>> +	EFI_GUID(0xde9f0ec, 0x88b6, 0x428f, 0x97, 0x7a, \
>>> +		 0x25, 0x8f, 0x1d, 0xe, 0x5e, 0x72)
>>> +
>>>    struct efi_capsule_header {
>>>    	efi_guid_t capsule_guid;
>>>    	u32 header_size;
>>> @@ -236,6 +240,19 @@ struct efi_capsule_result_variable_header {
>>>    	efi_status_t capsule_status;
>>>    } __packed;
>>>
>>> +struct efi_memory_range {
>>> +	efi_physical_addr_t	address;
>>> +	u64			length;
>>> +};
>>> +
>>> +struct efi_memory_range_capsule {
>>> +	struct efi_capsule_header *header;
>>> +	/* EFI_MEMORY_TYPE: 0x80000000-0xFFFFFFFF */
>>> +	enum efi_mem_type os_requested_memory_type;
>>> +	u64 number_of_memory_ranges;
>>> +	struct efi_memory_range memory_ranges[];
>>> +} __packed;
>>> +
>>>    #define EFI_RT_SUPPORTED_GET_TIME			0x0001
>>>    #define EFI_RT_SUPPORTED_SET_TIME			0x0002
>>>    #define EFI_RT_SUPPORTED_GET_WAKEUP_TIME		0x0004
>>>
>>
Takahiro Akashi March 18, 2020, 7:57 a.m. UTC | #4
On Wed, Mar 18, 2020 at 08:35:57AM +0100, Heinrich Schuchardt wrote:
> On 3/18/20 2:22 AM, AKASHI Takahiro wrote:
> > On Tue, Mar 17, 2020 at 09:11:43AM +0100, Heinrich Schuchardt wrote:
> > > On 3/17/20 3:12 AM, AKASHI Takahiro wrote:
> > > > Memory range capsule gives us a way to notify that some memory regions
> > > > should be left untouched across the next reset.
> > > > See UEFI specification, section 8.5.3.
> > > 
> > > There are multiple places where RAM availability is tested by writing
> > > during board bring up. Please, describe which changes are needed in ATF
> > > and U-Boot to make memory range capsules work reliably.
> > 
> > I think that they are quite system/platform-specific
> > and that it's beyond the scope of this patch.
> > 
> > Thanks,
> > -Takahiro Akashi
> 
> To me it does not make sense to merge this development if the platform
> compatibility is not evaluated.

What do you mean by "platform compatibility?"
Please elaborate it and specific requirements, if any,
for better understandings.

-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> > 
> > 
> > > Consider also memory encryption provided by some CPUs.
> > > https://www.tomshardware.com/news/intel-mktme-amd-memory-encryption,39467.html
> > > 
> > > Aren't file based capsules a much more reliable way for the OS to
> > > communicate to the firmware?
> > > 
> > > Best regards
> > > 
> > > Heinrich
> > > 
> > > > 
> > > > Since how we should handle this kind of capsule is totally up to
> > > > the system, no implementation will be added in this commit.
> > > > 
> > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > > ---
> > > >    include/efi_api.h | 17 +++++++++++++++++
> > > >    1 file changed, 17 insertions(+)
> > > > 
> > > > diff --git a/include/efi_api.h b/include/efi_api.h
> > > > index ac2b38801c0c..b7bf21cac7ad 100644
> > > > --- a/include/efi_api.h
> > > > +++ b/include/efi_api.h
> > > > @@ -221,6 +221,10 @@ enum efi_reset_type {
> > > >    	EFI_GUID(0x39b68c46, 0xf7fb, 0x441b, 0xb6, 0xec, \
> > > >    		 0x16, 0xb0, 0xf6, 0x98, 0x21, 0xf3)
> > > > 
> > > > +#define EFI_MEMORY_RANGE_CAPSULE_GUID \
> > > > +	EFI_GUID(0xde9f0ec, 0x88b6, 0x428f, 0x97, 0x7a, \
> > > > +		 0x25, 0x8f, 0x1d, 0xe, 0x5e, 0x72)
> > > > +
> > > >    struct efi_capsule_header {
> > > >    	efi_guid_t capsule_guid;
> > > >    	u32 header_size;
> > > > @@ -236,6 +240,19 @@ struct efi_capsule_result_variable_header {
> > > >    	efi_status_t capsule_status;
> > > >    } __packed;
> > > > 
> > > > +struct efi_memory_range {
> > > > +	efi_physical_addr_t	address;
> > > > +	u64			length;
> > > > +};
> > > > +
> > > > +struct efi_memory_range_capsule {
> > > > +	struct efi_capsule_header *header;
> > > > +	/* EFI_MEMORY_TYPE: 0x80000000-0xFFFFFFFF */
> > > > +	enum efi_mem_type os_requested_memory_type;
> > > > +	u64 number_of_memory_ranges;
> > > > +	struct efi_memory_range memory_ranges[];
> > > > +} __packed;
> > > > +
> > > >    #define EFI_RT_SUPPORTED_GET_TIME			0x0001
> > > >    #define EFI_RT_SUPPORTED_SET_TIME			0x0002
> > > >    #define EFI_RT_SUPPORTED_GET_WAKEUP_TIME		0x0004
> > > > 
> > > 
>
Takahiro Akashi April 6, 2020, 7:48 a.m. UTC | #5
Heinrich,

On Wed, Mar 18, 2020 at 04:57:16PM +0900, AKASHI Takahiro wrote:
> On Wed, Mar 18, 2020 at 08:35:57AM +0100, Heinrich Schuchardt wrote:
> > On 3/18/20 2:22 AM, AKASHI Takahiro wrote:
> > > On Tue, Mar 17, 2020 at 09:11:43AM +0100, Heinrich Schuchardt wrote:
> > > > On 3/17/20 3:12 AM, AKASHI Takahiro wrote:
> > > > > Memory range capsule gives us a way to notify that some memory regions
> > > > > should be left untouched across the next reset.
> > > > > See UEFI specification, section 8.5.3.
> > > > 
> > > > There are multiple places where RAM availability is tested by writing
> > > > during board bring up. Please, describe which changes are needed in ATF
> > > > and U-Boot to make memory range capsules work reliably.
> > > 
> > > I think that they are quite system/platform-specific
> > > and that it's beyond the scope of this patch.
> > > 
> > > Thanks,
> > > -Takahiro Akashi
> > 
> > To me it does not make sense to merge this development if the platform
> > compatibility is not evaluated.
> 
> What do you mean by "platform compatibility?"
> Please elaborate it and specific requirements, if any,
> for better understandings.

I need your feedback to improve my code.

-Takahiro Akashi

> -Takahiro Akashi
> 
> > Best regards
> > 
> > Heinrich
> > 
> > > 
> > > 
> > > > Consider also memory encryption provided by some CPUs.
> > > > https://www.tomshardware.com/news/intel-mktme-amd-memory-encryption,39467.html
> > > > 
> > > > Aren't file based capsules a much more reliable way for the OS to
> > > > communicate to the firmware?
> > > > 
> > > > Best regards
> > > > 
> > > > Heinrich
> > > > 
> > > > > 
> > > > > Since how we should handle this kind of capsule is totally up to
> > > > > the system, no implementation will be added in this commit.
> > > > > 
> > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > > > ---
> > > > >    include/efi_api.h | 17 +++++++++++++++++
> > > > >    1 file changed, 17 insertions(+)
> > > > > 
> > > > > diff --git a/include/efi_api.h b/include/efi_api.h
> > > > > index ac2b38801c0c..b7bf21cac7ad 100644
> > > > > --- a/include/efi_api.h
> > > > > +++ b/include/efi_api.h
> > > > > @@ -221,6 +221,10 @@ enum efi_reset_type {
> > > > >    	EFI_GUID(0x39b68c46, 0xf7fb, 0x441b, 0xb6, 0xec, \
> > > > >    		 0x16, 0xb0, 0xf6, 0x98, 0x21, 0xf3)
> > > > > 
> > > > > +#define EFI_MEMORY_RANGE_CAPSULE_GUID \
> > > > > +	EFI_GUID(0xde9f0ec, 0x88b6, 0x428f, 0x97, 0x7a, \
> > > > > +		 0x25, 0x8f, 0x1d, 0xe, 0x5e, 0x72)
> > > > > +
> > > > >    struct efi_capsule_header {
> > > > >    	efi_guid_t capsule_guid;
> > > > >    	u32 header_size;
> > > > > @@ -236,6 +240,19 @@ struct efi_capsule_result_variable_header {
> > > > >    	efi_status_t capsule_status;
> > > > >    } __packed;
> > > > > 
> > > > > +struct efi_memory_range {
> > > > > +	efi_physical_addr_t	address;
> > > > > +	u64			length;
> > > > > +};
> > > > > +
> > > > > +struct efi_memory_range_capsule {
> > > > > +	struct efi_capsule_header *header;
> > > > > +	/* EFI_MEMORY_TYPE: 0x80000000-0xFFFFFFFF */
> > > > > +	enum efi_mem_type os_requested_memory_type;
> > > > > +	u64 number_of_memory_ranges;
> > > > > +	struct efi_memory_range memory_ranges[];
> > > > > +} __packed;
> > > > > +
> > > > >    #define EFI_RT_SUPPORTED_GET_TIME			0x0001
> > > > >    #define EFI_RT_SUPPORTED_SET_TIME			0x0002
> > > > >    #define EFI_RT_SUPPORTED_GET_WAKEUP_TIME		0x0004
> > > > > 
> > > > 
> >
diff mbox series

Patch

diff --git a/include/efi_api.h b/include/efi_api.h
index ac2b38801c0c..b7bf21cac7ad 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -221,6 +221,10 @@  enum efi_reset_type {
 	EFI_GUID(0x39b68c46, 0xf7fb, 0x441b, 0xb6, 0xec, \
 		 0x16, 0xb0, 0xf6, 0x98, 0x21, 0xf3)
 
+#define EFI_MEMORY_RANGE_CAPSULE_GUID \
+	EFI_GUID(0xde9f0ec, 0x88b6, 0x428f, 0x97, 0x7a, \
+		 0x25, 0x8f, 0x1d, 0xe, 0x5e, 0x72)
+
 struct efi_capsule_header {
 	efi_guid_t capsule_guid;
 	u32 header_size;
@@ -236,6 +240,19 @@  struct efi_capsule_result_variable_header {
 	efi_status_t capsule_status;
 } __packed;
 
+struct efi_memory_range {
+	efi_physical_addr_t	address;
+	u64			length;
+};
+
+struct efi_memory_range_capsule {
+	struct efi_capsule_header *header;
+	/* EFI_MEMORY_TYPE: 0x80000000-0xFFFFFFFF */
+	enum efi_mem_type os_requested_memory_type;
+	u64 number_of_memory_ranges;
+	struct efi_memory_range memory_ranges[];
+} __packed;
+
 #define EFI_RT_SUPPORTED_GET_TIME			0x0001
 #define EFI_RT_SUPPORTED_SET_TIME			0x0002
 #define EFI_RT_SUPPORTED_GET_WAKEUP_TIME		0x0004