On 28/08/2024 3:56 pm, Jan Beulich wrote:
> On 28.08.2024 16:44, Andrew Cooper wrote:
>> On 27/08/2024 5:07 pm, Jan Beulich wrote:
>>> On 27.08.2024 15:57, Andrew Cooper wrote:
>>>> + for_each_set_bit ( seg, dirty )
>>>> + hvm_set_segment_register(curr, seg, &hvmemul_ctxt->seg_reg[seg]);
>>>> +
>>>> + hvmemul_ctxt->seg_reg_dirty = 0;
>>> Why is this suddenly appearing here? You don't mention it in the
>>> description,
>>> so it's not clear whether you found a (however minor) issue, or whether
>>> that's purely cosmetic (yet then it's still an extra store we could do
>>> without).
>> Oh, yes. Nothing anywhere in Xen ever clears these segment dirty bits.
> hvm_emulate_init_once()?
I meant after emulation. The value is initialised to 0 at the start of day.
>
>> I suspect the worst that will go wrong is that we'll waste time
>> re-{VMWRITE,memcpy}-ing the segment registers into the VMCS/VMCB, but
>> the logic in Xen is definitely not right.
> I'm on the edge of asking to do such clearing before emulation, not after
> processing the dirty bits. That would then be hvm_emulate_init_per_insn(),
> well centralized.
Specifically, hvmemul_ctxt should not believe itself to be dirty after a
call to hvm_emulate_writeback(), because that's the logic to make the
context no-longer-dirty.
That said, the more I look at this, the less convinced I am by it. For
a function named writeback(), it's doing a very narrow thing that is not
the usual meaning of the term when it comes to pipelines or insn
emulation...
~Andrew
