On Thu, 14 Nov 2019 06:38:46 +0100
Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> wrote:
> Em Wed, 13 Nov 2019 23:16:23 +0800
> Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> escreveu:
>
> > As CCIX Request Agents have fully cache coherent caches,
> > the CCIX 1.0 Base Specification defines detailed error
> > reporting for these caches.
> >
> > A CCIX cache error is reported via a CPER record as defined in the
> > UEFI 2.8 specification. The PER log section is defined in the
> > CCIX 1.0 Base Specification.
> >
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> > ---
...
> > +static int cper_ccix_cache_err_details(const char *pfx,
> > + struct acpi_hest_generic_data *gdata)
> > +{
> > + struct cper_ccix_cache_error *full_cache_err;
> > + struct cper_sec_ccix_cache_error *cache_err;
> > + u16 vendor_data_len;
> > + int i;
> > +
> > + if (gdata->error_data_length < sizeof(*full_cache_err))
> > + return -ENOSPC;
> > +
> > + full_cache_err = acpi_hest_get_payload(gdata);
> > +
> > + cache_err = &full_cache_err->cache_record;
> > +
> > + if (cache_err->validation_bits & CCIX_CACHE_ERR_TYPE_VALID)
> > + printk("%s""Cache Type: %s\n", pfx,
> > + cper_ccix_cache_type_str(cache_err->cache_type));
> > +
> > + if (cache_err->validation_bits & CCIX_CACHE_ERR_OP_VALID)
> > + printk("%s""Operation: %s\n", pfx,
> > + cper_ccix_cache_err_op_str(cache_err->op_type));
> > +
> > + if (cache_err->validation_bits & CCIX_CACHE_ERR_CACHE_ERR_TYPE_VALID)
> > + printk("%s""Cache Error Type: %s\n", pfx,
> > + cper_ccix_cache_err_type_str(cache_err->cache_error_type));
> > +
> > + if (cache_err->validation_bits & CCIX_CACHE_ERR_LEVEL_VALID)
> > + printk("%s""Level: %d\n", pfx, cache_err->cache_level);
> > +
> > + if (cache_err->validation_bits & CCIX_CACHE_ERR_SET_VALID)
> > + printk("%s""Set: %d\n", pfx, cache_err->set);
> > +
> > + if (cache_err->validation_bits & CCIX_CACHE_ERR_WAY_VALID)
> > + printk("%s""Way: %d\n", pfx, cache_err->way);
> > +
> > + if (cache_err->validation_bits & CCIX_CACHE_ERR_INSTANCE_ID_VALID)
> > + printk("%s""Instance ID: %d\n", pfx, cache_err->instance);
> > +
> > + if (cache_err->validation_bits & CCIX_CACHE_ERR_VENDOR_DATA_VALID) {
> > + if (gdata->error_data_length < sizeof(*full_cache_err) + 4)
> > + return -ENOSPC;
> > +
> > + vendor_data_len = cache_err->vendor_data[0] & GENMASK(15, 0);
> > + if (gdata->error_data_length <
> > + sizeof(*full_cache_err) + vendor_data_len)
> > + return -ENOSPC;
> > +
> > + for (i = 0; i < vendor_data_len / 4 - 1; i++)
> > + printk("%s""Vendor%d: 0x%08x\n", pfx, i,
> > + cache_err->vendor_data[i + 1]);
>
> I forgot to comment this at patch 1/6, as this is more a reflection
> than asking for a change...
>
> Not sure what's the value of also printing events to the Kernel logs.
>
> I mean, we do that for existent RAS drivers, mainly because the RAS report
> mechanism came after the printks, and someone could be relying at the
> kernel logs instead of using rasdaemon (or some other alternative software
> someone might write).
>
> For new report mechanisms, perhaps we could be smarter - at least offering
> ways to disable the printks if a daemon is listening to the trace events.
>
> Boris/Tony: what do you think?
>
Indeed, seems like a sensible time to make such a change if people agree
it makes sense to do so.
I'll leave this for now and get a v5 out with the fixes you mention.
> > + }
> > +
> > + return 0;
> > +}
> > +
....
>
>
> Cheers,
> Mauro
