On 07/01/2025 3:41 pm, Jan Beulich wrote:
> On 07.01.2025 16:37, Andrew Cooper wrote:
>> On 07/01/2025 2:33 pm, Jan Beulich wrote:
>>> All selector fields under ctxt->regs are (normally) poisoned in the HVM
>>> case, and the four ones besides CS and SS are potentially stale for PV.
>>> Avoid using them in the hypervisor incarnation of the emulator, when
>>> trying to cover for a missing ->read_segment() hook.
>>>
>>> To make sure there's always a valid ->read_segment() handler for all HVM
>>> cases, add a respective function to shadow code, even if it is not
>>> expected for FPU insns to be used to update page tables.
>>>
>>> Fixes: 0711b59b858a ("x86emul: correct FPU code/data pointers and opcode
>>> handling")
>>> Reported-by: Andrew Cooper <andrew.coop...@citrix.com>
>>> Signed-off-by: Jan Beulich <jbeul...@suse.com>
>>> ---
>>> The code comment may want adjusting in the course of FRED work.
>> It compiles when displacing my temporary patch in the FRED branch. I've
>> not got the ABI compatibility in userspace working yet, but
>> regs->{ds,es,fs,gs} will be staying, so the #else case should be fine
>> (assuming they're populated properly).
>>
>> So, tentatively, Acked-by: Andrew Cooper <andrew.coop...@citrix.com>
> Thanks.
>
>> That said, I think it would be nicer to see about excluding the FPU in
>> these cases. Both cases lacking read_segment() hooks are pagetable
>> emulation, and I'd say it's more likely to be code corruption than there
>> actually being x87 instructions in the middle of a dual 32bit PAE update.
> I considered this case, but decided against going this route. We shouldn't
> be stricter than necessary towards what we permit guests to do, however odd
> it might look to us.
I suppose so, but then I wonder if we ought to be setting up more
infrastructure by default for emulations.
We've got an awful lot of the emulator which has fallback paths upon
fallback paths, and probably is not as well tested as it ought to be.
~Andrew
