Re: [PATCH v12 2/3] cxl/core, EINJ: Add EINJ CXL debugfs files and EINJ helper functions

[Ben Cheatham <benjamin.cheatham@amd.com>]

On 2/15/24 3:33 AM, Jonathan Cameron wrote:
> On Wed, 14 Feb 2024 14:07:08 -0600
> Ben Cheatham <Benjamin.Cheatham@amd.com> wrote:
> 
>> Implement CXL helper functions in the EINJ module for getting/injecting
>> available CXL protocol error types and export them to sysfs under
>> kernel/debug/cxl.
>>
>> The kernel/debug/cxl/einj_types file will print the available CXL
>> protocol errors in the same format as the available_error_types
>> file provided by the EINJ module. The
>> kernel/debug/cxl/$dport_dev/einj_inject is functionally the same as the
>> error_type and error_inject files provided by the EINJ module, i.e.:
>> writing an error type into $dport_dev/einj_inject will inject said error
>> type into the CXL dport represented by $dport_dev.
>>
>> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
> 
> Hi Ben,
> 
> Just one trivial thing to add to Dan's comments.
> 
>> diff --git a/include/linux/einj-cxl.h b/include/linux/einj-cxl.h
>> new file mode 100644
>> index 000000000000..92c0e2e37ad9
>> --- /dev/null
>> +++ b/include/linux/einj-cxl.h
>> @@ -0,0 +1,40 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * CXL protocol Error INJection support.
>> + *
>> + * Copyright (c) 2023 Advanced Micro Devices, Inc.
>> + * All Rights Reserved.
>> + *
>> + * Author: Ben Cheatham <benjamin.cheatham@amd.com>
>> + */
>> +#ifndef CXL_EINJ_H
>> +#define CXL_EINJ_H
>> +
>> +#include <linux/pci.h>
>> +
>> +#if IS_ENABLED(CONFIG_ACPI_APEI_EINJ)
>> +int einj_cxl_available_error_type_show(struct seq_file *m, void *v);
>> +int einj_cxl_inject_error(struct pci_dev *dport_dev, u64 type);
>> +int einj_cxl_inject_rch_error(u64 rcrb, u64 type);
>> +bool einj_is_initialized(void);
>> +#else // !IS_ENABLED(CONFIG_ACPI_APEI_EINJ)
> Whilst C++ Style comments are allowed for a few specific case, you should stick
> to local style. In the include directory a quick grep gave me loads of
> #else /*  */
> lines and only 2
> #else //
> 
> So C style comments preferred.
> 

You are 100% correct. I did a grep for examples and didn't notice that I did it
from the base linux directory, so of course I found a bunch of examples of C++
style comments :/. I'll fix it in the next revision.

Thanks,
Ben

> 
>> +static inline int einj_cxl_available_error_type_show(struct seq_file *m,
>> +						     void *v)
>> +{
>> +	return -ENXIO;
>> +}
>> +
>> +static inline int einj_cxl_inject_error(struct pci_dev *dport_dev, u64 type)
>> +{
>> +	return -ENXIO;
>> +}
>> +
>> +static inline int einj_cxl_inject_rch_error(u64 rcrb, u64 type)
>> +{
>> +	return -ENXIO;
>> +}
>> +
>> +static inline bool einj_is_initialized(void) { return false; }
>> +#endif // CONFIG_ACPI_APEI_EINJ
>> +
>> +#endif // CXL_EINJ_H
>