[PATCH] tests/vpci: Use $(CC) instead of $(HOSTCC)

[PATCH] tests/vpci: Use $(CC) instead of $(HOSTCC)

[Michal Orzel]

These tests are supposed to run on target. HOSTCC can be different than
CC (when cross-compiling). At the moment, tests installation would put
a binary of a wrong format in the destdir.

Fixes: e90580f25bd7 ("vpci: introduce basic handlers to trap accesses to the PCI config space")
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
 tools/tests/vpci/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/tests/vpci/Makefile b/tools/tests/vpci/Makefile
index 9450f7593a41..1101a669e118 100644
--- a/tools/tests/vpci/Makefile
+++ b/tools/tests/vpci/Makefile
@@ -11,7 +11,7 @@ run: $(TARGET)
 	./$(TARGET)
 
 $(TARGET): vpci.c vpci.h list.h main.c emul.h
-	$(HOSTCC) $(CFLAGS_xeninclude) -g -o $@ vpci.c main.c
+	$(CC) $(CFLAGS_xeninclude) -g -o $@ vpci.c main.c
 
 .PHONY: clean
 clean:
-- 
2.25.1

Re: [PATCH] tests/vpci: Use $(CC) instead of $(HOSTCC)

[Andrew Cooper]

On 02/06/2025 4:09 pm, Michal Orzel wrote:
> These tests are supposed to run on target. HOSTCC can be different than
> CC (when cross-compiling). At the moment, tests installation would put
> a binary of a wrong format in the destdir.
>
> Fixes: e90580f25bd7 ("vpci: introduce basic handlers to trap accesses to the PCI config space")
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>

Oh.  This didn't explode in GitlabCI because there's no ARM version of
*-tools-tests-*.

Can we fix that too please, seeing as there is a real ARM board?

Also, I guess we have to finally sort out the CC vs HOSTCC debate.

~Andrew

Re: [PATCH] tests/vpci: Use $(CC) instead of $(HOSTCC)

[Orzel, Michal]

On 02/06/2025 17:36, Andrew Cooper wrote:
> On 02/06/2025 4:09 pm, Michal Orzel wrote:
>> These tests are supposed to run on target. HOSTCC can be different than
>> CC (when cross-compiling). At the moment, tests installation would put
>> a binary of a wrong format in the destdir.
>>
>> Fixes: e90580f25bd7 ("vpci: introduce basic handlers to trap accesses to the PCI config space")
>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> 
> Oh.  This didn't explode in GitlabCI because there's no ARM version of
> *-tools-tests-*.
> 
> Can we fix that too please, seeing as there is a real ARM board?
We will add it to our TODO.

> 
> Also, I guess we have to finally sort out the CC vs HOSTCC debate.
> 
> ~Andrew

~Michal

Re: [PATCH] tests/vpci: Use $(CC) instead of $(HOSTCC)

[Jan Beulich]

On 02.06.2025 17:36, Andrew Cooper wrote:
> On 02/06/2025 4:09 pm, Michal Orzel wrote:
>> These tests are supposed to run on target. HOSTCC can be different than
>> CC (when cross-compiling). At the moment, tests installation would put
>> a binary of a wrong format in the destdir.
>>
>> Fixes: e90580f25bd7 ("vpci: introduce basic handlers to trap accesses to the PCI config space")
>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> 
> Oh.  This didn't explode in GitlabCI because there's no ARM version of
> *-tools-tests-*.
> 
> Can we fix that too please, seeing as there is a real ARM board?
> 
> Also, I guess we have to finally sort out the CC vs HOSTCC debate.

I think the situation here makes pretty clear that HOSTCC is almost always
wrong to use for tests/. The emulator test harness is special in that it (in
principle) needs a target compiler (CC) and additionally an x86 one (with no
present representation). The present way of (partly) distinguishing the two
by using CC and HOSTCC was assigning wrong meaning to one (perhaps both) of
them. The (or maybe just my) problem is that in the toolchain world it is
build, host, and target which are distinguished. As per Michal's description
my understanding is that HOSTCC is matching "build" there, not "host".

Jan

Re: [PATCH] tests/vpci: Use $(CC) instead of $(HOSTCC)

[Andrew Cooper]

On 02/06/2025 4:50 pm, Jan Beulich wrote:
> On 02.06.2025 17:36, Andrew Cooper wrote:
>> On 02/06/2025 4:09 pm, Michal Orzel wrote:
>>> These tests are supposed to run on target. HOSTCC can be different than
>>> CC (when cross-compiling). At the moment, tests installation would put
>>> a binary of a wrong format in the destdir.
>>>
>>> Fixes: e90580f25bd7 ("vpci: introduce basic handlers to trap accesses to the PCI config space")
>>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
>> Oh.  This didn't explode in GitlabCI because there's no ARM version of
>> *-tools-tests-*.
>>
>> Can we fix that too please, seeing as there is a real ARM board?
>>
>> Also, I guess we have to finally sort out the CC vs HOSTCC debate.
> I think the situation here makes pretty clear that HOSTCC is almost always
> wrong to use for tests/. The emulator test harness is special in that it (in
> principle) needs a target compiler (CC) and additionally an x86 one (with no
> present representation). The present way of (partly) distinguishing the two
> by using CC and HOSTCC was assigning wrong meaning to one (perhaps both) of
> them. The (or maybe just my) problem is that in the toolchain world it is
> build, host, and target which are distinguished. As per Michal's description
> my understanding is that HOSTCC is matching "build" there, not "host".

vPCI is a true unit test.  It can be run in the build environment. 
Installing it here was to get it run somewhere.

x86_emulator is weird.  It can be run in the build environment, but it
is also dependent on CPUID so wants running on target environments too.

Everything else (IIRC) needs a real dom0 to run in, because they make
hypercalls.

~Andrew

Re: [PATCH] tests/vpci: Use $(CC) instead of $(HOSTCC)

[Orzel, Michal]

On 02/06/2025 17:50, Jan Beulich wrote:
> On 02.06.2025 17:36, Andrew Cooper wrote:
>> On 02/06/2025 4:09 pm, Michal Orzel wrote:
>>> These tests are supposed to run on target. HOSTCC can be different than
>>> CC (when cross-compiling). At the moment, tests installation would put
>>> a binary of a wrong format in the destdir.
>>>
>>> Fixes: e90580f25bd7 ("vpci: introduce basic handlers to trap accesses to the PCI config space")
>>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
>>
>> Oh.  This didn't explode in GitlabCI because there's no ARM version of
>> *-tools-tests-*.
>>
>> Can we fix that too please, seeing as there is a real ARM board?
>>
>> Also, I guess we have to finally sort out the CC vs HOSTCC debate.
> 
> I think the situation here makes pretty clear that HOSTCC is almost always
> wrong to use for tests/. The emulator test harness is special in that it (in
> principle) needs a target compiler (CC) and additionally an x86 one (with no
> present representation). The present way of (partly) distinguishing the two
> by using CC and HOSTCC was assigning wrong meaning to one (perhaps both) of
> them. The (or maybe just my) problem is that in the toolchain world it is
> build, host, and target which are distinguished. As per Michal's description
> my understanding is that HOSTCC is matching "build" there, not "host".
Yes, I found this issue while building latest Xen with Yocto. Build is x86, host
is Arm. HOSTCC is x86 gcc, CC is Arm gcc.

I take as this patch is waiting for a maintainer tag? Not that it needs to wait
for some generic cleanup?

~Michal

Re: [PATCH] tests/vpci: Use $(CC) instead of $(HOSTCC)

[Roger Pau Monné]

On Mon, Jun 02, 2025 at 05:09:29PM +0200, Michal Orzel wrote:
> These tests are supposed to run on target. HOSTCC can be different than
> CC (when cross-compiling). At the moment, tests installation would put
> a binary of a wrong format in the destdir.
> 
> Fixes: e90580f25bd7 ("vpci: introduce basic handlers to trap accesses to the PCI config space")
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> ---
>  tools/tests/vpci/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/tests/vpci/Makefile b/tools/tests/vpci/Makefile
> index 9450f7593a41..1101a669e118 100644
> --- a/tools/tests/vpci/Makefile
> +++ b/tools/tests/vpci/Makefile
> @@ -11,7 +11,7 @@ run: $(TARGET)
>  	./$(TARGET)
>  
>  $(TARGET): vpci.c vpci.h list.h main.c emul.h
> -	$(HOSTCC) $(CFLAGS_xeninclude) -g -o $@ vpci.c main.c
> +	$(CC) $(CFLAGS_xeninclude) -g -o $@ vpci.c main.c

This was already posted in:

https://lore.kernel.org/xen-devel/20230313121226.86557-1-roger.pau@citrix.com/

And got no feedback.

I'm happy for your change to go in, but you might also consider
picking up the run target adjustment part of that previous patch.

Thanks, Roger.

Re: [PATCH] tests/vpci: Use $(CC) instead of $(HOSTCC)

[Orzel, Michal]

On 03/06/2025 08:46, Roger Pau Monné wrote:
> On Mon, Jun 02, 2025 at 05:09:29PM +0200, Michal Orzel wrote:
>> These tests are supposed to run on target. HOSTCC can be different than
>> CC (when cross-compiling). At the moment, tests installation would put
>> a binary of a wrong format in the destdir.
>>
>> Fixes: e90580f25bd7 ("vpci: introduce basic handlers to trap accesses to the PCI config space")
>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
>> ---
>>  tools/tests/vpci/Makefile | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/tests/vpci/Makefile b/tools/tests/vpci/Makefile
>> index 9450f7593a41..1101a669e118 100644
>> --- a/tools/tests/vpci/Makefile
>> +++ b/tools/tests/vpci/Makefile
>> @@ -11,7 +11,7 @@ run: $(TARGET)
>>  	./$(TARGET)
>>  
>>  $(TARGET): vpci.c vpci.h list.h main.c emul.h
>> -	$(HOSTCC) $(CFLAGS_xeninclude) -g -o $@ vpci.c main.c
>> +	$(CC) $(CFLAGS_xeninclude) -g -o $@ vpci.c main.c
> 
> This was already posted in:
> 
> https://lore.kernel.org/xen-devel/20230313121226.86557-1-roger.pau@citrix.com/
> 
> And got no feedback.
> 
> I'm happy for your change to go in, but you might also consider
> picking up the run target adjustment part of that previous patch.
You're the maintainer of this file. You should tell me what I need to do
unless you want to wait for Anthony feedback.

~Michal

Re: [PATCH] tests/vpci: Use $(CC) instead of $(HOSTCC)

[Roger Pau Monné]

On Tue, Jun 03, 2025 at 08:52:38AM +0200, Orzel, Michal wrote:
> 
> 
> On 03/06/2025 08:46, Roger Pau Monné wrote:
> > On Mon, Jun 02, 2025 at 05:09:29PM +0200, Michal Orzel wrote:
> >> These tests are supposed to run on target. HOSTCC can be different than
> >> CC (when cross-compiling). At the moment, tests installation would put
> >> a binary of a wrong format in the destdir.
> >>
> >> Fixes: e90580f25bd7 ("vpci: introduce basic handlers to trap accesses to the PCI config space")

Hm, it's unclear to me whether this is the correct fixes tag.
Previous to:

96a587a05736 tools/tests: Add install target for vPCI

The test was not installed on the host, and hence didn't need to use
CC instead of HOSTCC (or at least that's my understating).

> >> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> >> ---
> >>  tools/tests/vpci/Makefile | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/tools/tests/vpci/Makefile b/tools/tests/vpci/Makefile
> >> index 9450f7593a41..1101a669e118 100644
> >> --- a/tools/tests/vpci/Makefile
> >> +++ b/tools/tests/vpci/Makefile
> >> @@ -11,7 +11,7 @@ run: $(TARGET)
> >>  	./$(TARGET)
> >>  
> >>  $(TARGET): vpci.c vpci.h list.h main.c emul.h
> >> -	$(HOSTCC) $(CFLAGS_xeninclude) -g -o $@ vpci.c main.c
> >> +	$(CC) $(CFLAGS_xeninclude) -g -o $@ vpci.c main.c
> > 
> > This was already posted in:
> > 
> > https://lore.kernel.org/xen-devel/20230313121226.86557-1-roger.pau@citrix.com/
> > 
> > And got no feedback.
> > 
> > I'm happy for your change to go in, but you might also consider
> > picking up the run target adjustment part of that previous patch.
> You're the maintainer of this file. You should tell me what I need to do
> unless you want to wait for Anthony feedback.

I would also add the chunk to adjust the run target if you use CC
instead of HOSTCC.

Thanks, Roger.

Re: [PATCH] tests/vpci: Use $(CC) instead of $(HOSTCC)

[Orzel, Michal]

On 03/06/2025 12:30, Roger Pau Monné wrote:
> On Tue, Jun 03, 2025 at 08:52:38AM +0200, Orzel, Michal wrote:
>>
>>
>> On 03/06/2025 08:46, Roger Pau Monné wrote:
>>> On Mon, Jun 02, 2025 at 05:09:29PM +0200, Michal Orzel wrote:
>>>> These tests are supposed to run on target. HOSTCC can be different than
>>>> CC (when cross-compiling). At the moment, tests installation would put
>>>> a binary of a wrong format in the destdir.
>>>>
>>>> Fixes: e90580f25bd7 ("vpci: introduce basic handlers to trap accesses to the PCI config space")
> 
> Hm, it's unclear to me whether this is the correct fixes tag.
> Previous to:
> 
> 96a587a05736 tools/tests: Add install target for vPCI
> 
> The test was not installed on the host, and hence didn't need to use
> CC instead of HOSTCC (or at least that's my understating).
You're right. Without install target, HOSTCC could make sense there.
I'll change the Fixes commit.

> 
>>>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
>>>> ---
>>>>  tools/tests/vpci/Makefile | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/tools/tests/vpci/Makefile b/tools/tests/vpci/Makefile
>>>> index 9450f7593a41..1101a669e118 100644
>>>> --- a/tools/tests/vpci/Makefile
>>>> +++ b/tools/tests/vpci/Makefile
>>>> @@ -11,7 +11,7 @@ run: $(TARGET)
>>>>  	./$(TARGET)
>>>>  
>>>>  $(TARGET): vpci.c vpci.h list.h main.c emul.h
>>>> -	$(HOSTCC) $(CFLAGS_xeninclude) -g -o $@ vpci.c main.c
>>>> +	$(CC) $(CFLAGS_xeninclude) -g -o $@ vpci.c main.c
>>>
>>> This was already posted in:
>>>
>>> https://lore.kernel.org/xen-devel/20230313121226.86557-1-roger.pau@citrix.com/
>>>
>>> And got no feedback.
>>>
>>> I'm happy for your change to go in, but you might also consider
>>> picking up the run target adjustment part of that previous patch.
>> You're the maintainer of this file. You should tell me what I need to do
>> unless you want to wait for Anthony feedback.
> 
> I would also add the chunk to adjust the run target if you use CC
> instead of HOSTCC.
Ok, will do.

~Michal