On Fri, Apr 18, 2025 at 01:01:55PM -0700, Luck, Tony wrote:
> On Thu, Apr 17, 2025 at 03:00:18PM -0700, Zaid Alali wrote:
> > + if (is_V2) {
> > + int count = 0, bytes_read, pos = 0, nr_parsed = 0, str_len;
> > + unsigned int comp, synd;
>
> I've been staring at the ACPI spec to try and figure out how many bits
> are needed for comp and sync. The example in section 18.6.7 "Error
> Injection Version 2 Operation" has this in step 6:
>
> Component Syndrome Array [0] = { 00000000000000000000000000000004 , 000000000000000000000000A5A5A5A5 }
>
> Which really looks like 128-bit values!
>
> So are "unsigned int" adequate? Even "u64" looks like it would fall short.
Hi Tony,
Hi Tony,
Thats a great point! I beleive I need to change "struct syndrom_array" to make all union members have
a 128-bit size
struct syndrome_array {
union {
u32 acpi_id; ====> all should be 128-bit long
u32 device_id;
u32 pcie_sbdf;
u8 vendor_id[16];
} comp_id;
union {
u32 proc_synd;
u32 mem_synd;
u32 pcie_synd;
u8 vendor_synd[16];
} comp_synd;
};
>
> > + struct syndrome_array *component_arr;
> > +
> > + component_arr = v5param->einjv2_struct.component_arr;
> > + str_len = strlen(user_input);
> > +
> > + while ((nr_parsed = sscanf(user_input + pos, "%x %x\n%n", &comp,
>
> Parsing user input with sscanf() is a bit fragile. Take a look at
> rdtgroup_schemata_write() which uses:
>
> while ((tok = strsep(&buf, "\n")) != NULL) {
>
> to split input into lines, and then strim() and strsep() to break
> up items within a line.
>
> > + &synd, &bytes_read))) {
> > + pos += bytes_read;
> > +
> > + if (nr_parsed != 2) {
> > + kfree(v5param);
> > + goto err_out;
> > + }
> > + if (count >= nr_components) {
> > + kfree(v5param);
> > + goto err_out;
> > + }
> > +
> > + switch (type) {
> > + case EINJV2_PROCESSOR_ERROR:
> > + component_arr[count].comp_id.acpi_id = comp;
> > + component_arr[count].comp_synd.proc_synd = synd;
> > + break;
> > + case EINJV2_MEMORY_ERROR:
> > + component_arr[count].comp_id.device_id = comp;
> > + component_arr[count].comp_synd.mem_synd = synd;
> > + break;
> > + case EINJV2_PCIE_ERROR:
> > + component_arr[count].comp_id.pcie_sbdf = comp;
> > + component_arr[count].comp_synd.pcie_synd = synd;
> > + break;
I also need to include another case here for EINJv2_VENDOR_ERROR
I will fix this in the next revision.
-Zaid
> > + }
> > + count++;
> > + if (pos >= str_len)
> > + break;
> > + }
