| Message ID | 20221206205234.606073-1-Benjamin.Cheatham@amd.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | ACPI, APEI, EINJ: Add new CXL Error Types | expand |
> Add new CXL error types so that they are advertised. > > Quick Note: > I sent out an email last week explaining why I was taking over this > patch series, but just in case: Jay's internship at AMD ended a couple > of months ago and I was asked to pick up this patch set for her. I also > said I was going to add a patch in aforementioned email, but I didn't > have a machine to test it so I've left it out. Thanks These look like a good start. But will there soon be changes to: drivers/acpi/apei/einj.c to do something with these new error types? -Tony
On 12/6/22 3:11 PM, Luck, Tony wrote: >> Add new CXL error types so that they are advertised. >> >> Quick Note: >> I sent out an email last week explaining why I was taking over this >> patch series, but just in case: Jay's internship at AMD ended a couple >> of months ago and I was asked to pick up this patch set for her. I also >> said I was going to add a patch in aforementioned email, but I didn't >> have a machine to test it so I've left it out. Thanks > These look like a good start. But will there soon be changes to: > > drivers/acpi/apei/einj.c > > to do something with these new error types? > > -Tony Hi Tony, The last patch I mentioned leaving out added support for injecting CXL errors, but I don't have access to a machine that I can test it with at the moment so it'll probably have to wait.
Hi Ben, > The last patch I mentioned leaving out added support for injecting CXL > errors, but I don't have access to a machine that I can test it with at > the moment so it'll probably have to wait. Parts 1 & 2 of your series can be applied now (as nice cleanups). But part 3 would just be confusing to users without the matching patch to add CXL injection support. I.e. a user might # cat /sys/kernel/debug/apei/einj/available_error_type and see: 0x00001000 CXL.cache Protocol Correctable But: # echo 0x1000 > /sys/kernel/debug/apei/einj/error_type wouldn't do anything useful (may do weird stuff since the driver doesn't appear to validate the "type" ... would be great if you fixed that while you are digging around in this code :-). So I'm happy to offer up a "Reviewed-by: Tony Luck <tony.luck@intel.com>" for all three parts. I just think that part 3 should not be applied until the rest of the code to go with it is ready. -Tony
On Tue, Dec 6, 2022 at 10:37 PM Luck, Tony <tony.luck@intel.com> wrote: > > Hi Ben, > > > The last patch I mentioned leaving out added support for injecting CXL > > errors, but I don't have access to a machine that I can test it with at > > the moment so it'll probably have to wait. > > Parts 1 & 2 of your series can be applied now (as nice cleanups). > > But part 3 would just be confusing to users without the matching patch > to add CXL injection support. > > I.e. a user might > > # cat /sys/kernel/debug/apei/einj/available_error_type > > and see: > > 0x00001000 CXL.cache Protocol Correctable > > But: > > # echo 0x1000 > /sys/kernel/debug/apei/einj/error_type > > wouldn't do anything useful (may do weird stuff since the driver > doesn't appear to validate the "type" ... would be great if you fixed > that while you are digging around in this code :-). > > So I'm happy to offer up a "Reviewed-by: Tony Luck <tony.luck@intel.com>" > for all three parts. I just think that part 3 should not be applied until the > rest of the code to go with it is ready. I agree, so I've done accordingly. Patches [1-2/3] have been applied as 6.2 material (with very minor subject adjustments) and Ben please resend patch [3/3] when sending the other material mentioned above (and please feel free to add the tag from Tony to it when doing that).
On 12/7/22 11:19 AM, Rafael J. Wysocki wrote: > On Tue, Dec 6, 2022 at 10:37 PM Luck, Tony <tony.luck@intel.com> wrote: >> Hi Ben, >> >>> The last patch I mentioned leaving out added support for injecting CXL >>> errors, but I don't have access to a machine that I can test it with at >>> the moment so it'll probably have to wait. >> Parts 1 & 2 of your series can be applied now (as nice cleanups). >> >> But part 3 would just be confusing to users without the matching patch >> to add CXL injection support. >> >> I.e. a user might >> >> # cat /sys/kernel/debug/apei/einj/available_error_type >> >> and see: >> >> 0x00001000 CXL.cache Protocol Correctable >> >> But: >> >> # echo 0x1000 > /sys/kernel/debug/apei/einj/error_type >> >> wouldn't do anything useful (may do weird stuff since the driver >> doesn't appear to validate the "type" ... would be great if you fixed >> that while you are digging around in this code :-). >> >> So I'm happy to offer up a "Reviewed-by: Tony Luck <tony.luck@intel.com>" >> for all three parts. I just think that part 3 should not be applied until the >> rest of the code to go with it is ready. > I agree, so I've done accordingly. > > Patches [1-2/3] have been applied as 6.2 material (with very minor > subject adjustments) and Ben please resend patch [3/3] when sending > the other material mentioned above (and please feel free to add the > tag from Tony to it when doing that). Will do. Thanks for the review!