Re: [PATCH 1/6 gnumach] Fix apic_send_ipi function clobbering read only fields

[Samuel Thibault]

Applied, thanks!

Damien Zammit, le lun. 05 févr. 2024 11:33:39 +0000, a ecrit:
> This was the root cause of failing to INIT.
> We were clobbering remote_read_status.
> And also, we need to reference the .r register
> when writing the ICR regs otherwise I think
> it writes all of the block.
> ---
>  i386/i386/apic.c | 8 ++++++--
>  i386/i386/apic.h | 2 +-
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/i386/i386/apic.c b/i386/i386/apic.c
> index 0cf7c37c..700fafd3 100644
> --- a/i386/i386/apic.c
> +++ b/i386/i386/apic.c
> @@ -290,6 +290,10 @@ void apic_send_ipi(unsigned dest_shorthand, unsigned 
> deliv_mode, unsigned dest_m
>      IcrLReg icrl_values;
>      IcrHReg icrh_values;
>  
> +    /* Keep previous values and only overwrite known fields */
> +    icrl_values.r = lapic->icr_low.r;
> +    icrh_values.r = lapic->icr_high.r;
> +
>      icrl_values.destination_shorthand = dest_shorthand;
>      icrl_values.delivery_mode = deliv_mode;
>      icrl_values.destination_mode = dest_mode;
> @@ -298,8 +302,8 @@ void apic_send_ipi(unsigned dest_shorthand, unsigned 
> deliv_mode, unsigned dest_m
>      icrl_values.vector = vector;
>      icrh_values.destination_field = dest_id;
>  
> -    lapic->icr_high = icrh_values;
> -    lapic->icr_low = icrl_values;
> +    lapic->icr_high.r = icrh_values.r;
> +    lapic->icr_low.r = icrl_values.r;
>  }
>  
>  void
> diff --git a/i386/i386/apic.h b/i386/i386/apic.h
> index e410e9c6..e1d49895 100644
> --- a/i386/i386/apic.h
> +++ b/i386/i386/apic.h
> @@ -83,7 +83,7 @@ typedef union u_icr_low
>          unsigned :1;
>          unsigned level: 1;
>          unsigned trigger_mode: 1;
> -        unsigned :2;
> +        unsigned remote_read_status: 2;      /* Read-only field */
>          unsigned destination_shorthand: 2;
>          unsigned :12;
>      };
> -- 
> 2.43.0
> 
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.