On 28/08/2024 10:00 am, Jan Beulich wrote:
> For partial writes the non-written parts of registers are folded into
> the full 64-bit value from what they're presently set to. That's wrong
> to do though when the behavior is write-1-to-clear: Writes not
> including to low 3 bits would unconditionally clear all ISR bits which
> are presently set. Re-calculate the value to use.
>
> Fixes: be07023be115 ("x86/vhpet: add support for level triggered interrupts")
> Signed-off-by: Jan Beulich <[email protected]>
> ---
> Alternatively we could also suppress the folding in of read bits, but
> that looked to me to end up in a more intrusive change.
>
> --- a/xen/arch/x86/hvm/hpet.c
> +++ b/xen/arch/x86/hvm/hpet.c
> @@ -404,7 +404,8 @@ static int cf_check hpet_write(
> break;
>
> case HPET_STATUS:
> - /* write 1 to clear. */
> + /* Write 1 to clear. Therefore don't use new_val directly here. */
> + new_val = val << ((addr & 7) * 8);
> while ( new_val )
> {
> bool active;
It's sad that we allow any sub-word accesses. The spec makes it pretty
clear that only aligned 32-bit and 64-bit accesses are acceptable, and
it would simplify the merging logic substantially.
Nevertheless, this is the simplest bugfix for now.
Reviewed-by: Andrew Cooper <[email protected]>
