| 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 |
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 >
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 > > >
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 >>> >>
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 > > > > > > > >
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 --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
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(+)