Re: [PATCH v3 4/6] hw/cxl: QMP based poison injection support

[Jonathan Cameron]

> > +    memset(out, 0, out_pl_len);
> > +    QLIST_FOREACH(ent, poison_list, node) {
> > +        uint64_t start, stop;
> > +
> > +        /* Check for no overlap */
> > +        if (ent->start >= query_start + query_length ||
> > +            ent->start + ent->length <= query_start) {
> > +            continue;
> > +        }
> > +
> > +        /* Deal with overlap */
> > +        start = MAX(ent->start & 0xffffffffffffffc0, query_start);
> > +        stop = MIN((ent->start & 0xffffffffffffffc0) + ent->length,
> > +                   query_start + query_length);
> > +        stq_le_p(&out->records[i].addr, start | (ent->type & 0x3));  
> 
> Shouldn't the mask here be 0x7?  I see we have not define Vendor Specific
> which I think is good but maybe better to allow it here?  I'm just not
> sure what is going to happen if someone comes along later and wants to
> use that value.

As things stand there is no way to inject a vendor defined poison entry
so expanding the mask has no affect.  However, it'll be one less change needed
if we ever add that support to the qmp interface so I'll make the change
as I'm respinning for your other feedback.

Given we've not supported anything vendor defined yet, I'm not that bothered
about adding the qmp support any time soon ;)

+ where else can poison come from?  It's either in the device or it's not
or it doesn't exist and was injected.  I'll be interested to see the arguement
for another source unless vendor defined really means 'I don't know'.

Thanks for the review and eagle eyed register checking :)

Jonathan