mbox series

[RFC,v3,0/3] Add support for Vendor Defined Error Types in Einj Module

Message ID 20230612215139.5132-1-Avadhut.Naik@amd.com
Headers
Series Add support for Vendor Defined Error Types in Einj Module |

Message

Avadhut Naik June 12, 2023, 9:51 p.m. UTC 
  This patchset adds support for Vendor Defined Error types in the einj
module by exporting a binary blob file in module's debugfs directory.
Userspace tools can write OEM Defined Structures into the blob file as
part of injecting Vendor defined errors.

The first patch refactors available_error_type_show() function to ensure
all errors supported by the platform are output through einj module's
available_error_type file in debugfs.

The second patch adds a write callback for binary blobs created through
debugfs_create_blob() API.

The third adds the required support i.e. establishing the memory mapping
and exporting it through debugfs blob file for Vendor-defined Error types.

Changes in v2:
 - Split the v1 patch, as was recommended, to have a separate patch for
changes in debugfs.
 - Refactored available_error_type_show() function into a separate patch.
 - Changed file permissions to octal format to remove checkpatch warnings.

Changes in v3:
 - Use BIT macro for generating error masks instead of hex values since
ACPI spec uses bit numbers.
 - Handle the corner case of acpi_os_map_iomem() returning NULL through
a local variable to a store the size of OEM defined data structure.

Avadhut Naik (3):
  ACPI: APEI: EINJ: Refactor available_error_type_show()
  fs: debugfs: Add write functionality to debugfs blobs
  ACPI: APEI: EINJ: Add support for vendor defined error types

 drivers/acpi/apei/einj.c | 67 +++++++++++++++++++++++++++-------------
 fs/debugfs/file.c        | 28 ++++++++++++++---
 2 files changed, 69 insertions(+), 26 deletions(-)

Comments

Greg KH June 13, 2023, 8:01 a.m. UTC | #1 
On Mon, Jun 12, 2023 at 09:51:36PM +0000, Avadhut Naik wrote:
> This patchset adds support for Vendor Defined Error types in the einj
> module by exporting a binary blob file in module's debugfs directory.
> Userspace tools can write OEM Defined Structures into the blob file as
> part of injecting Vendor defined errors.
> 
> The first patch refactors available_error_type_show() function to ensure
> all errors supported by the platform are output through einj module's
> available_error_type file in debugfs.
> 
> The second patch adds a write callback for binary blobs created through
> debugfs_create_blob() API.
> 
> The third adds the required support i.e. establishing the memory mapping
> and exporting it through debugfs blob file for Vendor-defined Error types.
> 
> Changes in v2:
>  - Split the v1 patch, as was recommended, to have a separate patch for
> changes in debugfs.
>  - Refactored available_error_type_show() function into a separate patch.
>  - Changed file permissions to octal format to remove checkpatch warnings.
> 
> Changes in v3:
>  - Use BIT macro for generating error masks instead of hex values since
> ACPI spec uses bit numbers.
>  - Handle the corner case of acpi_os_map_iomem() returning NULL through
> a local variable to a store the size of OEM defined data structure.
> 
> Avadhut Naik (3):
>   ACPI: APEI: EINJ: Refactor available_error_type_show()
>   fs: debugfs: Add write functionality to debugfs blobs
>   ACPI: APEI: EINJ: Add support for vendor defined error types
> 
>  drivers/acpi/apei/einj.c | 67 +++++++++++++++++++++++++++-------------
>  fs/debugfs/file.c        | 28 ++++++++++++++---
>  2 files changed, 69 insertions(+), 26 deletions(-)
> 
> -- 
> 2.34.1
> 

Why is a RFC series at v3?  What is left to be done with it to make you
confident that it can be merged?

I almost never review RFC patches as obviously the submitter doesn't
think it is good enough to be reviewed, and hundreds of other patches in
my review queue are from people who think they are ready to be merged,
so this puts your stuff always at the bottom of the list...

When submitting something with "RFC" ask what type of comments you are
looking for and why you do not think this is ready yet, otherwise we
have no idea...

thanks,

greg k-h
Naik, Avadhut June 13, 2023, 10:34 p.m. UTC | #2 
On 6/13/2023 03:01, Greg KH wrote:
> On Mon, Jun 12, 2023 at 09:51:36PM +0000, Avadhut Naik wrote:
>> This patchset adds support for Vendor Defined Error types in the einj
>> module by exporting a binary blob file in module's debugfs directory.
>> Userspace tools can write OEM Defined Structures into the blob file as
>> part of injecting Vendor defined errors.
>>
>> The first patch refactors available_error_type_show() function to ensure
>> all errors supported by the platform are output through einj module's
>> available_error_type file in debugfs.
>>
>> The second patch adds a write callback for binary blobs created through
>> debugfs_create_blob() API.
>>
>> The third adds the required support i.e. establishing the memory mapping
>> and exporting it through debugfs blob file for Vendor-defined Error types.
>>
>> Changes in v2:
>>  - Split the v1 patch, as was recommended, to have a separate patch for
>> changes in debugfs.
>>  - Refactored available_error_type_show() function into a separate patch.
>>  - Changed file permissions to octal format to remove checkpatch warnings.
>>
>> Changes in v3:
>>  - Use BIT macro for generating error masks instead of hex values since
>> ACPI spec uses bit numbers.
>>  - Handle the corner case of acpi_os_map_iomem() returning NULL through
>> a local variable to a store the size of OEM defined data structure.
>>
>> Avadhut Naik (3):
>>   ACPI: APEI: EINJ: Refactor available_error_type_show()
>>   fs: debugfs: Add write functionality to debugfs blobs
>>   ACPI: APEI: EINJ: Add support for vendor defined error types
>>
>>  drivers/acpi/apei/einj.c | 67 +++++++++++++++++++++++++++-------------
>>  fs/debugfs/file.c        | 28 ++++++++++++++---
>>  2 files changed, 69 insertions(+), 26 deletions(-)
>>
>> -- 
>> 2.34.1
>>
> 
> Why is a RFC series at v3?  What is left to be done with it to make you
> confident that it can be merged?
> 
	Wasn't very confident of the debugfs changes since the binary blobs
created through debugfs_create_blob() have been read-only for a considerable
amount of time. Was wondering if there were some known issues in making them
writable. So, wanted to seek opinion on the changes while also incorporating
the feedback received. Having said that, since you confirmed that you are okay
with the debugfs changes, will remove the RFC tag in subsequent revision.
Apologies for the confusion and inconvenience caused, if any.

> I almost never review RFC patches as obviously the submitter doesn't
> think it is good enough to be reviewed, and hundreds of other patches in
> my review queue are from people who think they are ready to be merged,
> so this puts your stuff always at the bottom of the list...
> 
> When submitting something with "RFC" ask what type of comments you are
> looking for and why you do not think this is ready yet, otherwise we
> have no idea...
>
	Thank you so much for patiently clearing that up! Will surely keep
this in mind for the next time.
 
Thanks,
Avadhut Naik

> thanks,
> 
> greg k-h

--