Re: [PATCH v3 08/17] riscv: Introduce vendor variants of extension helpers

[Conor Dooley]

On Fri, Apr 26, 2024 at 01:01:10PM -0700, Charlie Jenkins wrote:
> On Fri, Apr 26, 2024 at 05:19:59PM +0100, Conor Dooley wrote:
> > On Sat, Apr 20, 2024 at 06:04:40PM -0700, Charlie Jenkins wrote:
> > > @@ -163,6 +164,8 @@ void thead_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
> > >  	for (alt = begin; alt < end; alt++) {
> > >  		if (alt->vendor_id != THEAD_VENDOR_ID)
> > >  			continue;
> > > +		if (alt->patch_id >= RISCV_VENDOR_EXT_ALTERNATIVES_BASE)
> > > +			continue;
> > 
> > >  		if (alt->patch_id >= ERRATA_THEAD_NUMBER)
> > 
> > This number is 2, how does the patching actually work for vendor stuff
> > when the base is always greater than 2?
> > 
> 
> Patching is handled through the patcher at the end of cpufeature.c. The
> vendor_id field is set in the alternatives for errata and now also for
> vendor extensions. The vendor extension patching is all handled
> generically.
> 
> This is distinguished by the patch_id being greater than
> RISCV_VENDOR_EXT_ALTERNATIVES_BASE, which should leave way more than
> enough room for errata ids. Since the code already checks if the
> patch_id is greater than the errata number, I can drop the
> "if (alt->patch_id >= RISCV_VENDOR_EXT_ALTERNATIVES_BASE)" check.

Ah, ye, I was being dumb - I was somehow thinking that you were using
the errata patch function to patch this, which obviously wouldn't be a
good design. If that had been the case, you'd never patch anything, but
you patch the extensions using the cpufeature patch function so it all
works out.

If anything, I guess there could be an assert here somewhere that
enforces ERRATA_VENDOR_NUMBER < VENDOR_EXT_BASE.

Attachment: signature.asc
Description: PGP signature