| Message ID | 20180516154233.21457-8-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Alexander Graf |
| Headers | show |
| Series | efi: Enable basic sandbox support for EFI loader | expand |
On 16.05.18 17:42, Simon Glass wrote: > With sandbox these values depend on the host system. Let's assume that it > is x86_64 for now. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > Changes in v4: None > Changes in v3: None > Changes in v2: None > > include/config_distro_bootcmd.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h > index 8d5feb3ac77..97d6baab4be 100644 > --- a/include/config_distro_bootcmd.h > +++ b/include/config_distro_bootcmd.h > @@ -246,7 +246,7 @@ > #elif defined(CONFIG_ARM) > #define BOOTENV_EFI_PXE_ARCH "0xa" > #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00010:UNDI:003000" > -#elif defined(CONFIG_X86) > +#elif defined(CONFIG_X86) || defined(CONFIG_SANDBOX) Why not #elif defined(CONFIG_X86) || (defined(CONFIG_SANDBOX) && defined(__x86_64__)) and similar for other architectures? That way we should be quite safe in determining our target architecture, no? Alex > /* Always assume we're running 64bit */ > #define BOOTENV_EFI_PXE_ARCH "0x7" > #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00007:UNDI:003000" >
Hi Alex, On 24 May 2018 at 06:32, Alexander Graf <agraf@suse.de> wrote: > > > On 16.05.18 17:42, Simon Glass wrote: >> With sandbox these values depend on the host system. Let's assume that it >> is x86_64 for now. >> >> Signed-off-by: Simon Glass <sjg@chromium.org> >> --- >> >> Changes in v4: None >> Changes in v3: None >> Changes in v2: None >> >> include/config_distro_bootcmd.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h >> index 8d5feb3ac77..97d6baab4be 100644 >> --- a/include/config_distro_bootcmd.h >> +++ b/include/config_distro_bootcmd.h >> @@ -246,7 +246,7 @@ >> #elif defined(CONFIG_ARM) >> #define BOOTENV_EFI_PXE_ARCH "0xa" >> #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00010:UNDI:003000" >> -#elif defined(CONFIG_X86) >> +#elif defined(CONFIG_X86) || defined(CONFIG_SANDBOX) > > Why not > > #elif defined(CONFIG_X86) || (defined(CONFIG_SANDBOX) && > defined(__x86_64__)) > > and similar for other architectures? That way we should be quite safe in > determining our target architecture, no? I suspect that would work, although I think it would need to be done centrally, rather than ad-hoc in files that need to know the sandbox host architecture. We are not currently aware of the sandbox host architecture, but I wonder whether we are going to have to teach the build system about it. Does U-Boot sandbox actually run on ARM platforms? I have not tried it. Regards, Simon
On 12.06.18 07:27, Simon Glass wrote: > Hi Alex, > > On 24 May 2018 at 06:32, Alexander Graf <agraf@suse.de> wrote: >> >> >> On 16.05.18 17:42, Simon Glass wrote: >>> With sandbox these values depend on the host system. Let's assume that it >>> is x86_64 for now. >>> >>> Signed-off-by: Simon Glass <sjg@chromium.org> >>> --- >>> >>> Changes in v4: None >>> Changes in v3: None >>> Changes in v2: None >>> >>> include/config_distro_bootcmd.h | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h >>> index 8d5feb3ac77..97d6baab4be 100644 >>> --- a/include/config_distro_bootcmd.h >>> +++ b/include/config_distro_bootcmd.h >>> @@ -246,7 +246,7 @@ >>> #elif defined(CONFIG_ARM) >>> #define BOOTENV_EFI_PXE_ARCH "0xa" >>> #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00010:UNDI:003000" >>> -#elif defined(CONFIG_X86) >>> +#elif defined(CONFIG_X86) || defined(CONFIG_SANDBOX) >> >> Why not >> >> #elif defined(CONFIG_X86) || (defined(CONFIG_SANDBOX) && >> defined(__x86_64__)) >> >> and similar for other architectures? That way we should be quite safe in >> determining our target architecture, no? > > I suspect that would work, although I think it would need to be done > centrally, rather than ad-hoc in files that need to know the sandbox > host architecture. > > We are not currently aware of the sandbox host architecture, but I > wonder whether we are going to have to teach the build system about > it. Does U-Boot sandbox actually run on ARM platforms? I have not > tried it. I haven't tried it either, but IMHO it's a bug if it doesn't run :). I also don't quite understand why CONFIG_SANDBOX contradicts CONFIG_X86. They probably should both be set for an x86 host system. That way you wouldn't have to double-check these conditionals all over the code base. Alex
Hi Alex, On 11 June 2018 at 23:42, Alexander Graf <agraf@suse.de> wrote: > > > On 12.06.18 07:27, Simon Glass wrote: >> Hi Alex, >> >> On 24 May 2018 at 06:32, Alexander Graf <agraf@suse.de> wrote: >>> >>> >>> On 16.05.18 17:42, Simon Glass wrote: >>>> With sandbox these values depend on the host system. Let's assume that it >>>> is x86_64 for now. >>>> >>>> Signed-off-by: Simon Glass <sjg@chromium.org> >>>> --- >>>> >>>> Changes in v4: None >>>> Changes in v3: None >>>> Changes in v2: None >>>> >>>> include/config_distro_bootcmd.h | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h >>>> index 8d5feb3ac77..97d6baab4be 100644 >>>> --- a/include/config_distro_bootcmd.h >>>> +++ b/include/config_distro_bootcmd.h >>>> @@ -246,7 +246,7 @@ >>>> #elif defined(CONFIG_ARM) >>>> #define BOOTENV_EFI_PXE_ARCH "0xa" >>>> #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00010:UNDI:003000" >>>> -#elif defined(CONFIG_X86) >>>> +#elif defined(CONFIG_X86) || defined(CONFIG_SANDBOX) >>> >>> Why not >>> >>> #elif defined(CONFIG_X86) || (defined(CONFIG_SANDBOX) && >>> defined(__x86_64__)) >>> >>> and similar for other architectures? That way we should be quite safe in >>> determining our target architecture, no? >> >> I suspect that would work, although I think it would need to be done >> centrally, rather than ad-hoc in files that need to know the sandbox >> host architecture. >> >> We are not currently aware of the sandbox host architecture, but I >> wonder whether we are going to have to teach the build system about >> it. Does U-Boot sandbox actually run on ARM platforms? I have not >> tried it. > > I haven't tried it either, but IMHO it's a bug if it doesn't run :). Well until someone tests it, it doesn't work. Any changes made to attempt to make it work without testing are just conjecture, IMO. > > I also don't quite understand why CONFIG_SANDBOX contradicts CONFIG_X86. > They probably should both be set for an x86 host system. That way you > wouldn't have to double-check these conditionals all over the code base. The point here is that they are separate U-Boot architectures. What I'm getting at is that in U-Boot we have the concept of a host architecture and we can use HOSTCC. But with sandbox this takes on a lot more meaning, since the host architecture is actually the one on which sandbox is running. So you could say that we have ARM sandbox or x86_64 sandbox. At present we have not such concept. I can see that it could be useful as things get more complicated. I suppose whoever tries U-Boot sandbox out on ARM first will hit there problems and send a patch? Regards, Simon
diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h index 8d5feb3ac77..97d6baab4be 100644 --- a/include/config_distro_bootcmd.h +++ b/include/config_distro_bootcmd.h @@ -246,7 +246,7 @@ #elif defined(CONFIG_ARM) #define BOOTENV_EFI_PXE_ARCH "0xa" #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00010:UNDI:003000" -#elif defined(CONFIG_X86) +#elif defined(CONFIG_X86) || defined(CONFIG_SANDBOX) /* Always assume we're running 64bit */ #define BOOTENV_EFI_PXE_ARCH "0x7" #define BOOTENV_EFI_PXE_VCI "PXEClient:Arch:00007:UNDI:003000"
With sandbox these values depend on the host system. Let's assume that it is x86_64 for now. Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v4: None Changes in v3: None Changes in v2: None include/config_distro_bootcmd.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)