kvm_s390_update_topology_change_report() modifies a single bit within
sca_utility using cmpxchg(). Given that the size of the sca_utility union
is two bytes this generates very inefficient code. Change the size to four
bytes, so better code can be generated.

Even though the size of sca_utility doesn't reflect architecture anymore
this seems to be the easiest and most pragmatic approach to avoid
inefficient code.

Acked-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/include/asm/kvm_host.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 1cd8eaebd3c0..1cb1de232b9e 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -95,10 +95,10 @@ union ipte_control {
 };
 
 union sca_utility {
-	__u16 val;
+	__u32 val;
 	struct {
-		__u16 mtcr : 1;
-		__u16 reserved : 15;
+		__u32 mtcr : 1;
+		__u32	   : 31;
 	};
 };
 
@@ -107,7 +107,7 @@ struct bsca_block {
 	__u64	reserved[5];
 	__u64	mcn;
 	union sca_utility utility;
-	__u8	reserved2[6];
+	__u8	reserved2[4];
 	struct bsca_entry cpu[KVM_S390_BSCA_CPU_SLOTS];
 };
 
@@ -115,7 +115,7 @@ struct esca_block {
 	union ipte_control ipte_control;
 	__u64   reserved1[6];
 	union sca_utility utility;
-	__u8	reserved2[6];
+	__u8	reserved2[4];
 	__u64   mcn[4];
 	__u64   reserved3[20];
 	struct esca_entry cpu[KVM_S390_ESCA_CPU_SLOTS];
-- 
2.45.2

On 11/26/24 11:25 AM, Heiko Carstens wrote:
[...]
>   union sca_utility {

Would you mind adding a comment?


""Utility is defined as 2 bytes but having it 4 bytes wide generates 
more efficient code. Since the following bytes are reserved this makes 
no functional difference.""

> -	__u16 val;
> +	__u32 val;
>   	struct {
> -		__u16 mtcr : 1;
> -		__u16 reserved : 15;
> +		__u32 mtcr : 1;
> +		__u32	   : 31;
>   	};
>   };
>   
> @@ -107,7 +107,7 @@ struct bsca_block {
>   	__u64	reserved[5];
>   	__u64	mcn;
>   	union sca_utility utility;
> -	__u8	reserved2[6];
> +	__u8	reserved2[4];
>   	struct bsca_entry cpu[KVM_S390_BSCA_CPU_SLOTS];
>   };
>   
> @@ -115,7 +115,7 @@ struct esca_block {
>   	union ipte_control ipte_control;
>   	__u64   reserved1[6];
>   	union sca_utility utility;
> -	__u8	reserved2[6];
> +	__u8	reserved2[4];
>   	__u64   mcn[4];
>   	__u64   reserved3[20];
>   	struct esca_entry cpu[KVM_S390_ESCA_CPU_SLOTS];

On Tue, 26 Nov 2024 13:09:56 +0100
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 11/26/24 11:25 AM, Heiko Carstens wrote:
> [...]
> >   union sca_utility {  
> 
> Would you mind adding a comment?
> 
> 
> ""Utility is defined as 2 bytes but having it 4 bytes wide generates 
> more efficient code. Since the following bytes are reserved this makes 
> no functional difference.""

looks good, thanks!

> 
> > -	__u16 val;
> > +	__u32 val;
> >   	struct {
> > -		__u16 mtcr : 1;
> > -		__u16 reserved : 15;
> > +		__u32 mtcr : 1;
> > +		__u32	   : 31;
> >   	};
> >   };
> >   
> > @@ -107,7 +107,7 @@ struct bsca_block {
> >   	__u64	reserved[5];
> >   	__u64	mcn;
> >   	union sca_utility utility;
> > -	__u8	reserved2[6];
> > +	__u8	reserved2[4];
> >   	struct bsca_entry cpu[KVM_S390_BSCA_CPU_SLOTS];
> >   };
> >   
> > @@ -115,7 +115,7 @@ struct esca_block {
> >   	union ipte_control ipte_control;
> >   	__u64   reserved1[6];
> >   	union sca_utility utility;
> > -	__u8	reserved2[6];
> > +	__u8	reserved2[4];
> >   	__u64   mcn[4];
> >   	__u64   reserved3[20];
> >   	struct esca_entry cpu[KVM_S390_ESCA_CPU_SLOTS];  
>

On Tue, Nov 26, 2024 at 01:21:52PM +0100, Claudio Imbrenda wrote:
> On Tue, 26 Nov 2024 13:09:56 +0100
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
> > On 11/26/24 11:25 AM, Heiko Carstens wrote:
> > [...]
> > >   union sca_utility {  
> > 
> > Would you mind adding a comment?
> > 
> > 
> > ""Utility is defined as 2 bytes but having it 4 bytes wide generates 
> > more efficient code. Since the following bytes are reserved this makes 
> > no functional difference.""
> 
> looks good, thanks!

Thanks a lot! I added the comment and applied the series.

On Tue, 26 Nov 2024 11:25:15 +0100
Heiko Carstens <hca@linux.ibm.com> wrote:

> kvm_s390_update_topology_change_report() modifies a single bit within
> sca_utility using cmpxchg(). Given that the size of the sca_utility union
> is two bytes this generates very inefficient code. Change the size to four
> bytes, so better code can be generated.
> 
> Even though the size of sca_utility doesn't reflect architecture anymore
> this seems to be the easiest and most pragmatic approach to avoid
> inefficient code.
> 
> Acked-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  arch/s390/include/asm/kvm_host.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index 1cd8eaebd3c0..1cb1de232b9e 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -95,10 +95,10 @@ union ipte_control {
>  };
>  
>  union sca_utility {
> -	__u16 val;
> +	__u32 val;

I know I said the patch was fine but I realised now that I would like a
short comment here explaining that 32 bits allows for more efficient
code

you can add it when picking, no need to send a v3

>  	struct {
> -		__u16 mtcr : 1;
> -		__u16 reserved : 15;
> +		__u32 mtcr : 1;
> +		__u32	   : 31;
>  	};
>  };
>  
> @@ -107,7 +107,7 @@ struct bsca_block {
>  	__u64	reserved[5];
>  	__u64	mcn;
>  	union sca_utility utility;
> -	__u8	reserved2[6];
> +	__u8	reserved2[4];
>  	struct bsca_entry cpu[KVM_S390_BSCA_CPU_SLOTS];
>  };
>  
> @@ -115,7 +115,7 @@ struct esca_block {
>  	union ipte_control ipte_control;
>  	__u64   reserved1[6];
>  	union sca_utility utility;
> -	__u8	reserved2[6];
> +	__u8	reserved2[4];
>  	__u64   mcn[4];
>  	__u64   reserved3[20];
>  	struct esca_entry cpu[KVM_S390_ESCA_CPU_SLOTS];