Re: [PATCH v5 15/20] hw/i386: update vapic_write to use MemTxAttrs

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 11/11/22 19:25, Alex Bennée wrote:
> This allows us to drop the current_cpu hack and properly model an
> invalid access to the vapic.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   hw/i386/kvmvapic.c | 19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
> index 43f8a8f679..a76ed07199 100644
> --- a/hw/i386/kvmvapic.c
> +++ b/hw/i386/kvmvapic.c
> @@ -635,20 +635,21 @@ static int vapic_prepare(VAPICROMState *s)
>       return 0;
>   }
>   
> -static void vapic_write(void *opaque, hwaddr addr, uint64_t data,
> -                        unsigned int size)
> +static MemTxResult vapic_write(void *opaque, hwaddr addr, uint64_t data,
> +                               unsigned int size, MemTxAttrs attrs)
>   {
>       VAPICROMState *s = opaque;
> +    CPUState *cs;
>       X86CPU *cpu;
>       CPUX86State *env;
>       hwaddr rom_paddr;
>   
> -    if (!current_cpu) {
> -        return;
> +    if (attrs.requester_type != MTRT_CPU) {
> +        return MEMTX_ACCESS_ERROR;
>       }
> -
> -    cpu_synchronize_state(current_cpu);
> -    cpu = X86_CPU(current_cpu);
> +    cs = qemu_get_cpu(attrs.requester_id);
> +    cpu_synchronize_state(cs);
> +    cpu = X86_CPU(cs);
>       env = &cpu->env;
>   
>       /*
> @@ -708,6 +709,8 @@ static void vapic_write(void *opaque, hwaddr addr, uint64_t data,
>           }
>           break;
>       }
> +
> +    return MEMTX_OK;
>   }
>   
>   static uint64_t vapic_read(void *opaque, hwaddr addr, unsigned size)
> @@ -716,7 +719,7 @@ static uint64_t vapic_read(void *opaque, hwaddr addr, unsigned size)
>   }
>   
>   static const MemoryRegionOps vapic_ops = {
> -    .write = vapic_write,
> +    .write_with_attrs = vapic_write,
>       .read = vapic_read,

Shouldn't we do the same for the read() path?

>       .endianness = DEVICE_NATIVE_ENDIAN,
>   };