| Message ID | 5f7afa11-3216-4175-b05b-3ff78920fa00@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/PV: use altcall for I/O emulation quirk hook | expand |
On 16/01/2024 4:58 pm, Jan Beulich wrote: > This way we can arrange for ioemul_handle_proliant_quirk()'s ENDBR to > also be zapped. Utilize existing data rather than introducing another > otherwise unused static variable (array); eventually (if any new quirk > was in need of adding) we may want to use .callback and .driver_data > anyway. > > For the decision to be taken before the 2nd alternative patching pass, > the initcall needs to become a pre-SMP one. > > While touching this code, also arrange for it to not be built at all > when !PV - that way the respective ENDBR won't be there from the > beginning. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > Obviously the file may want moving to pv/ then. I wasn't sure whether > to also fold doing so right into here. For PVH dom0, we allow almost blanket IO port access. We could do the same for PV dom0 by setting up a suitable TSS IO port bitmap. That said, x86-S is soon to revoke the ability to do that, so maybe we just save ourselves the work... I'm confused about "rather than introducing another otherwise unused static variable (array)". Why an array? In this instance, you could use the same trick as the ctxt switch mask. Whether we match DMI or not, it's safe to clobber the ENDBR. We could also consider a __{read_mostly,ro_after_init}_cf_clobber sections. However, it's probably better still to have a `bool prolient_quirk` and a direct call. No extra vendor hooks have been added since this was introduced in 2007, and I really don't foresee this changing in the near future. Lets just simplify it and drop all the alternatives/clobbering games entirely. ~Andrew
On 16.01.2024 18:31, Andrew Cooper wrote: > On 16/01/2024 4:58 pm, Jan Beulich wrote: >> This way we can arrange for ioemul_handle_proliant_quirk()'s ENDBR to >> also be zapped. Utilize existing data rather than introducing another >> otherwise unused static variable (array); eventually (if any new quirk >> was in need of adding) we may want to use .callback and .driver_data >> anyway. >> >> For the decision to be taken before the 2nd alternative patching pass, >> the initcall needs to become a pre-SMP one. >> >> While touching this code, also arrange for it to not be built at all >> when !PV - that way the respective ENDBR won't be there from the >> beginning. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> --- >> Obviously the file may want moving to pv/ then. I wasn't sure whether >> to also fold doing so right into here. > > For PVH dom0, we allow almost blanket IO port access. We could do the > same for PV dom0 by setting up a suitable TSS IO port bitmap. > > That said, x86-S is soon to revoke the ability to do that, so maybe we > just save ourselves the work... > > > I'm confused about "rather than introducing another otherwise unused > static variable (array)". Why an array? (Again) in anticipation of there being a need for another such quirk. Imo that would have been only consistent with the use of a function pointer. However, ... > In this instance, you could use the same trick as the ctxt switch mask. > Whether we match DMI or not, it's safe to clobber the ENDBR. We could > also consider a __{read_mostly,ro_after_init}_cf_clobber sections. > > > However, it's probably better still to have a `bool prolient_quirk` and > a direct call. No extra vendor hooks have been added since this was > introduced in 2007, and I really don't foresee this changing in the near > future. Lets just simplify it and drop all the alternatives/clobbering > games entirely. ... I've now done this. Will send a v2 soon. Jan
--- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -45,7 +45,7 @@ obj-$(CONFIG_LIVEPATCH) += alternative.o obj-y += msi.o obj-y += msr.o obj-$(CONFIG_INDIRECT_THUNK) += indirect-thunk.o -obj-y += ioport_emulate.o +obj-$(CONFIG_PV) += ioport_emulate.o obj-y += irq.o obj-$(CONFIG_KEXEC) += machine_kexec.o obj-y += mm.o x86_64/mm.o --- a/xen/arch/x86/ioport_emulate.c +++ b/xen/arch/x86/ioport_emulate.c @@ -36,7 +36,7 @@ static unsigned int cf_check ioemul_hand } /* This table is the set of system-specific I/O emulation hooks. */ -static const struct dmi_system_id __initconstrel ioport_quirks_tbl[] = { +static const struct dmi_system_id __initconst_cf_clobber ioport_quirks_tbl[] = { /* * I/O emulation hook for certain HP ProLiant servers with * 'special' SMM goodness. @@ -46,6 +46,8 @@ static const struct dmi_system_id __init DMI_MATCH2( DMI_MATCH(DMI_BIOS_VENDOR, "HP"), DMI_MATCH(DMI_PRODUCT_NAME, "ProLiant DL3")), + /* Need in one entry only as long as .callback isn't also used. */ + .driver_data = ioemul_handle_proliant_quirk, }, { .ident = "HP ProLiant DL5xx", @@ -99,7 +101,7 @@ static int __init cf_check ioport_quirks return 0; } -__initcall(ioport_quirks_init); +presmp_initcall(ioport_quirks_init); /* * Local variables: --- a/xen/arch/x86/pv/emul-priv-op.c +++ b/xen/arch/x86/pv/emul-priv-op.c @@ -112,7 +112,8 @@ static io_emul_stub_t *io_emul_stub_setu /* Some platforms might need to quirk the stub for specific inputs. */ if ( unlikely(ioemul_handle_quirk) ) { - quirk_bytes = ioemul_handle_quirk(opcode, p, ctxt->ctxt.regs); + quirk_bytes = alternative_call(ioemul_handle_quirk, opcode, p, + ctxt->ctxt.regs); p += quirk_bytes; }
This way we can arrange for ioemul_handle_proliant_quirk()'s ENDBR to also be zapped. Utilize existing data rather than introducing another otherwise unused static variable (array); eventually (if any new quirk was in need of adding) we may want to use .callback and .driver_data anyway. For the decision to be taken before the 2nd alternative patching pass, the initcall needs to become a pre-SMP one. While touching this code, also arrange for it to not be built at all when !PV - that way the respective ENDBR won't be there from the beginning. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- Obviously the file may want moving to pv/ then. I wasn't sure whether to also fold doing so right into here.