[v1] x86/hyperv: change hv_tlb_flush_ex to fix clang build

[Xen-devel] [PATCH] x86/hyperv: change hv_tlb_flush_ex to fix clang build

Posted by Wei Liu 4 years, 4 months ago

Clang complains:

In file included from synic.c:15:
/builds/xen-project/xen/xen/include/asm/guest/hyperv-tlfs.h:900:18: error: field 'hv_vp_set' with variable sized type 'struct hv_vpset' not at the end of a struct or class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
        struct hv_vpset hv_vp_set;
                        ^
1 error generated.
/builds/xen-project/xen/xen/Rules.mk:198: recipe for target 'synic.o' failed
make[6]: *** [synic.o] Error 1

Drop the last variable size array from hv_tlb_flush_ex for now to fix
clang builds.

Signed-off-by: Wei Liu <liuwe@microsoft.com>
---
 xen/include/asm-x86/guest/hyperv-tlfs.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/xen/include/asm-x86/guest/hyperv-tlfs.h b/xen/include/asm-x86/guest/hyperv-tlfs.h
index ccd9850b27..98ff1adc61 100644
--- a/xen/include/asm-x86/guest/hyperv-tlfs.h
+++ b/xen/include/asm-x86/guest/hyperv-tlfs.h
@@ -893,12 +893,11 @@ struct hv_tlb_flush {
 	u64 gva_list[];
 } __packed;
 
-/* HvFlushVirtualAddressSpaceEx, HvFlushVirtualAddressListEx hypercalls */
+/* HvFlushVirtualAddressSpaceEx hypercall */
 struct hv_tlb_flush_ex {
 	u64 address_space;
 	u64 flags;
 	struct hv_vpset hv_vp_set;
-	u64 gva_list[];
 } __packed;
 
 struct hv_partition_assist_pg {
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86/hyperv: change hv_tlb_flush_ex to fix clang build

Posted by Jan Beulich 4 years, 4 months ago

On 22.12.2019 00:50, Wei Liu wrote:
> Clang complains:
> 
> In file included from synic.c:15:
> /builds/xen-project/xen/xen/include/asm/guest/hyperv-tlfs.h:900:18: error: field 'hv_vp_set' with variable sized type 'struct hv_vpset' not at the end of a struct or class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
>         struct hv_vpset hv_vp_set;
>                         ^

I'm surprised gcc does not complain.

> 1 error generated.
> /builds/xen-project/xen/xen/Rules.mk:198: recipe for target 'synic.o' failed
> make[6]: *** [synic.o] Error 1
> 
> Drop the last variable size array from hv_tlb_flush_ex for now to fix
> clang builds.

Why "for now"? The field is illegal there, and I can't see a way to
make it legal. To know its actual place in memory you need to know
the number of elements in bank_contents[], i.e. having the field
if the compiler erroneously accepted it would mean wrong code to
get generated (silently).

> --- a/xen/include/asm-x86/guest/hyperv-tlfs.h
> +++ b/xen/include/asm-x86/guest/hyperv-tlfs.h
> @@ -893,12 +893,11 @@ struct hv_tlb_flush {
>  	u64 gva_list[];
>  } __packed;
>  
> -/* HvFlushVirtualAddressSpaceEx, HvFlushVirtualAddressListEx hypercalls */
> +/* HvFlushVirtualAddressSpaceEx hypercall */
>  struct hv_tlb_flush_ex {
>  	u64 address_space;
>  	u64 flags;
>  	struct hv_vpset hv_vp_set;
> -	u64 gva_list[];

Rather than outright removing the line, keep it as a comment?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86/hyperv: change hv_tlb_flush_ex to fix clang build

Posted by Wei Liu 4 years, 4 months ago

On Mon, Dec 23, 2019 at 10:53:29AM +0100, Jan Beulich wrote:
> On 22.12.2019 00:50, Wei Liu wrote:
> > Clang complains:
> > 
> > In file included from synic.c:15:
> > /builds/xen-project/xen/xen/include/asm/guest/hyperv-tlfs.h:900:18: error: field 'hv_vp_set' with variable sized type 'struct hv_vpset' not at the end of a struct or class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
> >         struct hv_vpset hv_vp_set;
> >                         ^
> 
> I'm surprised gcc does not complain.

Me too.

> 
> > 1 error generated.
> > /builds/xen-project/xen/xen/Rules.mk:198: recipe for target 'synic.o' failed
> > make[6]: *** [synic.o] Error 1
> > 
> > Drop the last variable size array from hv_tlb_flush_ex for now to fix
> > clang builds.
> 
> Why "for now"? The field is illegal there, and I can't see a way to
> make it legal. To know its actual place in memory you need to know
> the number of elements in bank_contents[], i.e. having the field
> if the compiler erroneously accepted it would mean wrong code to
> get generated (silently).
> 

OK. I can drop "for now".

> > --- a/xen/include/asm-x86/guest/hyperv-tlfs.h
> > +++ b/xen/include/asm-x86/guest/hyperv-tlfs.h
> > @@ -893,12 +893,11 @@ struct hv_tlb_flush {
> >  	u64 gva_list[];
> >  } __packed;
> >  
> > -/* HvFlushVirtualAddressSpaceEx, HvFlushVirtualAddressListEx hypercalls */
> > +/* HvFlushVirtualAddressSpaceEx hypercall */
> >  struct hv_tlb_flush_ex {
> >  	u64 address_space;
> >  	u64 flags;
> >  	struct hv_vpset hv_vp_set;
> > -	u64 gva_list[];
> 
> Rather than outright removing the line, keep it as a comment?

That's fine by me.

Wei.

> 
> Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel