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
