[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 2/2] xen: Populate xen.lds.h and make use of its macros



Hi,

On 30/03/2022 14:24, Michal Orzel wrote:


On 30.03.2022 14:53, Jan Beulich wrote:
On 30.03.2022 14:13, Michal Orzel wrote:
On 30.03.2022 13:57, Jan Beulich wrote:
On 30.03.2022 13:04, Michal Orzel wrote:
On 30.03.2022 12:42, Jan Beulich wrote:
On 30.03.2022 12:32, Julien Grall wrote:
Renaming to PE_COFF may help to avoid the confusion with CONFIG_EFI.
That said, it would possibly make more difficult to associate the flag
with "linking an EFI binary".

Indeed. And EFI_PE_COFF is getting a little unwieldy for my taste.

I think some documentaion about the define EFI would be help so there
are no more confusion between CONFIG_EFI/EFI. But I am not sure where to
put it. Maybe at the top of the header?

That's perhaps the best place, yes.

In this case how about the following comment at the top of xen.lds.h:

"To avoid any confusion about EFI macro used in this header vs EFI support,
the former is used when linking a native EFI (i.e. PE/COFF) binary, whereas
the latter means support for generating EFI binary.

No, that's the case on Arm only. As Julien suggested, it is perhaps best
to explain the difference between EFI and CONFIG_EFI, without going into
arch specifics.
Could you please tell me what you are reffering to as there is no such 
identifier
in Xen (as opposed to Linux) like CONFIG_EFI ?

Let's call it a "virtual" CONFIG_EFI then; I think we really should have
such an option. But yes, if you don't like referring to such a virtual
option, then describing what is meant verbally is certainly going to be
fine.

FWICS, there was an attempt done by Wei in his NUMA series to define CONFIG_EFI.
However as this is not yet merged and agreed, I would like not to refer to 
identifiers
not existing in the code, even though most people are familiar with this option 
from Linux.

So by taking an example from Linux I would verbally explain it like that:
"To avoid any confusion, please note that EFI macro does not correspond to EFI
runtime support and is used when linking a native EFI (i.e. PE/COFF) binary, 
hence its

"EFI runtime support" can be mistakenly associated to EFI runtime services (which BTW not supported on Arm). So I would suggest to s/runtime/boot/.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.