Re: Fonts and theming and what to do in future with SB
Hi, On 2022/12/1 00:08, Robbie Harwood wrote: Zhang Boyang writes: On 2022/11/30 02:35, Steve McIntyre wrote: AFAIK Chris Coulson has a patch for the font loader to cause it to try loading fonts from the embedded memdisk first. Is that the best approach? If so, what fonts should we be embedding in the signed image? It's a tradeoff between size and functionality, of course - some people are happy with just "unicode" while others may want a wider choice for added theming options. Is the size an issue for most people? Personally I prefer to embed Unifont to GRUB. This can be done by memdisk, or embed as code directly (like ASCII chars). For Unifont, this will make GRUB about 2.5MB (+150%) fatter. This is sufficient for vast majority of GRUB usecases. My testing with Chris's patch and approach for unicode.pf2 (in Fedora) adds just under a megabyte - presumably this is due to benefits from xz-compressing it, minus overhead to add the modules required for support to our builds. Is your suggestion that we should officially remove support for other fonts, then? It's also possible to embed only hashes of font files into GRUB (although not implemented yet). Therefore, distros can pre-generate a series of fonts files and add their hashes to GRUB bundle. End-users can choose their font in the pre-defined list from distros. As you say, someone would have to implement this first :) And if we were to go this route, I think we'd want to do the same for background images. For those who want to use fonts which not in the pre-defined list. The last option would be disable secureboot or use MOK to sign their own GRUB bundle. "Disable secureboot in order to do this" is not a position I ever want to take. In a forced tradeoff between features and security, some users will opt for features. (And it's easier to just disable secureboot than roll out one's own signing, unfortunately.) So my preference is to keep feature parity between SB and non-SB, even if that means dropping customizability. Or... Could/should we look at options to sign fonts separately? I've heard suggestions to embed them into faked-up modules that we could load with insmod, but of course we don't support signing modules yet anyway... :-) It seems "shim" can process PE files only, so it's hard to implement signature checking for other files (modules, fonts, etc.). A tradeoff solution is to wrap PF2 font into a PE file, so they can be processed by shim. A better solution would be add more functionality to "shim" to allow it process more types of files. I don't agree with making shim more complicated. Then I think the best solution would be: wrap font files into PE files. Therefore we can sign font files directly and verify them using "shim", and there is no need to use memdisks or something like that. If I understand correctly, user can also sign their own font files with MOK using existing tools and no special infrastructure need to be introduced. I'm planning to implement a PE unwrapper using `grub_file_filters`. Using it we can wrap even more type of files into PE files, like ELF files. I will send a RFC patch in few days. Be well, --Robbie Best Regards, Zhang Boyang ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2
On 12/1/22 00:19, Glenn Washburn wrote:
On Wed, 30 Nov 2022 17:42:40 -0500
Stefan Berger wrote:
On 11/30/22 16:24, Stefan Berger wrote:
On 11/30/22 14:47, Stefan Berger wrote:
On 11/24/22 12:56, Daniel Kiper wrote:
Hi,
Adding Sudhakar and Glenn...
On Thu, Aug 11, 2022 at 02:40:58PM -0300, Diego Domingos wrote:
Hello,
This is an addition to the series sent from Daniel Axtens
(https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html).
Patch 'ieee1275: request memory with
ibm,client-architecture-support' implements vectors 1-4 of
client-architecture-support negotiation However, during some
tests, we found this can be a problem if:
- we have more than 64 CPUs
- Hardware Management Console (HMC) is configured to minimum of
CPUs >64 (for example, min of 200 CPUs)
- Grub needs to request memory.
If vector 5 is not implemented, Power Hypervisor will consider
the default value for vector 5 and 64 will bet set as the
maximum number of CPUs supported by the OS, causing the machine
to fail to init. Today we support 256 CPUs (max) on Power, so we
need to implement vector 5 and set the MAX CPUs bits to this
value.
The patches 11-15 aren't merged to the grub tree yet, so I'm
sending those patches again together with my patch to implement
vector 5 on top of them.
The patches 11-15 contains the following:
Daniel Axtens (4):
ieee1275: request memory with ibm,client-architecture-support
ieee1275: drop len -= 1 quirk in heap_init
ieee1275: support runtime memory claiming
[RFC] Add memtool module with memory allocation stress-test
Stefan Berger (1):
ibmvtpm: Add support for trusted boot using a vTPM 2.0
I went through all patches and cannot see major problems with
them. Though there are a lot of minor things which have to be
fixed. Sadly due to number of them I cannot simply ignore that.
Here is the list of the issues:
- functions calls/sizeof(): e.g. "grub_printf()" should be
replaced with "grub_printf ()", add space before "(", in the
code; though I am OK with the former in comments and commit
messages,
- casts: e.g. "*(grub_uint32_t *)data" should be replaced with
"*(grub_uint32_t *) data", add space between ")" and "data",
- s/__attribute__((packed))/GRUB_PACKED/
- if you use grub_err_t type please test for GRUB_ERR_NONE
instead of !err or err; please do not use plain numbers, e.g. 0
to substitute GRUB_ERR_NONE,
- if you test pointers for NULL please test using NULL
constant instead of e.g. !ptr
- if you use a value often please define constant for it; good
candidate for such change is at least 0x3000 in the patch #3;
if constant definition is an overkill please comment what given
numbers/strings mean or at least where they come from,
- please do not use "//" for comments,
- I am OK with lines a bit longer than 80; so, please do not
wrap lines too early,
This is a bit vague but I think I addressed them now.
- year in the copyright should be 2022.
The GRUB coding style is described here [1] and you can find good
example of coding style in the grub-core/kern/efi/sb.c file.
Please take into account latest comments from Daniel A. and Glenn
too.
I don't know how to support the memtool without --enable-mm-debug
at the same time since the module seems to be missing then but the
build system still expects it on 'make install'. Unless there's an
existing example of how to do it I would not post with this patch.
I can get it to create an empty module with this trick here but
don't know whether this helps the cause.
GRUB_MOD_FINI (memtools)
{
#ifdef MM_DEBUG
grub_unregister_command (cmd_lsmem);
grub_unregister_command (cmd_lsfreemem);
grub_unregister_command (cmd_sba);
#else
(void) grub_unregister_command;
#endif
}
In 1/6 we have this here. Is this sufficiently gating the usage of
the code or do we need to use '#if defined(__powerpc__)' to only
compile code newly added powerpc-specific code used due to this
flag being set?
+
+ if (grub_strncmp (tmp, "IBM,", 4) == 0)
+ grub_ieee1275_set_flag
(GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY);
And yet another question: Is __i386__ actually using
grub-core/kern/ieee1275/init.c ? I don't see it compiling this file
but there's a #ifdef __i386__ in this file.
Yes, there is a i386-ieee1275 target. It builds and the tests run
successfully, iirc.
How is this target enabled? Just configuring on an i386 host doesn't seem to do
it and I don't see an obvious configure option to build for it, either.
Stefan
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2
Adding Vladimir...
On Wed, Nov 30, 2022 at 02:47:41PM -0500, Stefan Berger wrote:
> On 11/24/22 12:56, Daniel Kiper wrote:
> > Hi,
> >
> > Adding Sudhakar and Glenn...
> >
> > On Thu, Aug 11, 2022 at 02:40:58PM -0300, Diego Domingos wrote:
> > > Hello,
> > >
> > > This is an addition to the series sent from Daniel Axtens
> > > (https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html).
> > >
> > > Patch 'ieee1275: request memory with ibm,client-architecture-support'
> > > implements vectors 1-4 of client-architecture-support negotiation
> > > However, during some tests, we found this can be a problem if:
> > >
> > > - we have more than 64 CPUs
> > > - Hardware Management Console (HMC) is configured to minimum of CPUs >64
> > > (for example, min of 200 CPUs)
> > > - Grub needs to request memory.
> > >
> > > If vector 5 is not implemented, Power Hypervisor will consider the
> > > default value for vector 5 and 64 will bet set as the maximum
> > > number of CPUs supported by the OS, causing the machine to fail to init.
> > > Today we support 256 CPUs (max) on Power, so we need to implement vector
> > > 5 and set the MAX CPUs bits to this value.
> > >
> > > The patches 11-15 aren't merged to the grub tree yet, so I'm sending
> > > those patches again together with my patch to implement vector 5
> > > on top of them.
> > >
> > > The patches 11-15 contains the following:
> > >
> > > Daniel Axtens (4):
> > >ieee1275: request memory with ibm,client-architecture-support
> > >ieee1275: drop len -= 1 quirk in heap_init
> > >ieee1275: support runtime memory claiming
> > >[RFC] Add memtool module with memory allocation stress-test
> > >
> > > Stefan Berger (1):
> > >ibmvtpm: Add support for trusted boot using a vTPM 2.0
> >
> > I went through all patches and cannot see major problems with them.
> > Though there are a lot of minor things which have to be fixed. Sadly due
> > to number of them I cannot simply ignore that.
> >
> > Here is the list of the issues:
> >- functions calls/sizeof(): e.g. "grub_printf()" should be replaced with
> > "grub_printf ()",
> > add space before "(", in the code; though I am OK with the former in
> > comments and
> > commit messages,
> >- casts: e.g. "*(grub_uint32_t *)data" should be replaced with
> > "*(grub_uint32_t *) data",
> > add space between ")" and "data",
> >- s/__attribute__((packed))/GRUB_PACKED/
> >- if you use grub_err_t type please test for GRUB_ERR_NONE instead of
> > !err or err;
> > please do not use plain numbers, e.g. 0 to substitute GRUB_ERR_NONE,
> >- if you test pointers for NULL please test using NULL constant instead
> > of e.g. !ptr
> >- if you use a value often please define constant for it; good candidate
> > for such
> > change is at least 0x3000 in the patch #3; if constant definition
> > is an overkill
> > please comment what given numbers/strings mean or at least where they
> > come from,
> >- please do not use "//" for comments,
> >- I am OK with lines a bit longer than 80; so, please do not wrap
> > lines too early,
>
> This is a bit vague but I think I addressed them now.
Cool! Thanks!
> >- year in the copyright should be 2022.
> >
> > The GRUB coding style is described here [1] and you can find good
> > example of coding style in the grub-core/kern/efi/sb.c file.
> >
> > Please take into account latest comments from Daniel A. and Glenn too.
>
> I don't know how to support the memtool without --enable-mm-debug at the same
> time since the module seems to be missing then but the build system still
> expects it on 'make install'. Unless there's an existing example of how to do
> it I would not post with this patch.
>
> I can get it to create an empty module with this trick here but don't know
> whether this helps the cause.
>
> GRUB_MOD_FINI (memtools)
> {
> #ifdef MM_DEBUG
> grub_unregister_command (cmd_lsmem);
> grub_unregister_command (cmd_lsfreemem);
> grub_unregister_command (cmd_sba);
> #else
> (void) grub_unregister_command;
> #endif
> }
I think it would be better if you fix Makefile to not install non-existing
modules than use trick mentioned above. And it seems to me we should have
similar case somewhere to steal idea from. Or maybe Vladimir will have an
idea how to easily/nicely fix this issue. Vladimir?
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2
On Wed, Nov 30, 2022 at 04:24:59PM -0500, Stefan Berger wrote:
> On 11/30/22 14:47, Stefan Berger wrote:
> > On 11/24/22 12:56, Daniel Kiper wrote:
> > > Hi,
> > >
> > > Adding Sudhakar and Glenn...
> > >
> > > On Thu, Aug 11, 2022 at 02:40:58PM -0300, Diego Domingos wrote:
> > > > Hello,
> > > >
> > > > This is an addition to the series sent from Daniel Axtens
> > > > (https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html).
> > > >
> > > > Patch 'ieee1275: request memory with ibm,client-architecture-support'
> > > > implements vectors 1-4 of client-architecture-support negotiation
> > > > However, during some tests, we found this can be a problem if:
> > > >
> > > > - we have more than 64 CPUs
> > > > - Hardware Management Console (HMC) is configured to minimum of CPUs
> > > > >64 (for example, min of 200 CPUs)
> > > > - Grub needs to request memory.
> > > >
> > > > If vector 5 is not implemented, Power Hypervisor will consider the
> > > > default value for vector 5 and 64 will bet set as the maximum
> > > > number of CPUs supported by the OS, causing the machine to fail to init.
> > > > Today we support 256 CPUs (max) on Power, so we need to implement
> > > > vector 5 and set the MAX CPUs bits to this value.
> > > >
> > > > The patches 11-15 aren't merged to the grub tree yet, so I'm sending
> > > > those patches again together with my patch to implement vector 5
> > > > on top of them.
> > > >
> > > > The patches 11-15 contains the following:
> > > >
> > > > Daniel Axtens (4):
> > > > ieee1275: request memory with ibm,client-architecture-support
> > > > ieee1275: drop len -= 1 quirk in heap_init
> > > > ieee1275: support runtime memory claiming
> > > > [RFC] Add memtool module with memory allocation stress-test
> > > >
> > > > Stefan Berger (1):
> > > > ibmvtpm: Add support for trusted boot using a vTPM 2.0
> > >
> > > I went through all patches and cannot see major problems with them.
> > > Though there are a lot of minor things which have to be fixed. Sadly due
> > > to number of them I cannot simply ignore that.
> > >
> > > Here is the list of the issues:
> > > - functions calls/sizeof(): e.g. "grub_printf()" should be replaced
> > > with "grub_printf ()",
> > > add space before "(", in the code; though I am OK with the former in
> > > comments and
> > > commit messages,
> > > - casts: e.g. "*(grub_uint32_t *)data" should be replaced with
> > > "*(grub_uint32_t *) data",
> > > add space between ")" and "data",
> > > - s/__attribute__((packed))/GRUB_PACKED/
> > > - if you use grub_err_t type please test for GRUB_ERR_NONE instead of
> > > !err or err;
> > > please do not use plain numbers, e.g. 0 to substitute GRUB_ERR_NONE,
> > > - if you test pointers for NULL please test using NULL constant
> > > instead of e.g. !ptr
> > > - if you use a value often please define constant for it; good
> > > candidate for such
> > > change is at least 0x3000 in the patch #3; if constant
> > > definition is an overkill
> > > please comment what given numbers/strings mean or at least where
> > > they come from,
> > > - please do not use "//" for comments,
> > > - I am OK with lines a bit longer than 80; so, please do not wrap
> > > lines too early,
> >
> > This is a bit vague but I think I addressed them now.
> >
> > > - year in the copyright should be 2022.
> > >
> > > The GRUB coding style is described here [1] and you can find good
> > > example of coding style in the grub-core/kern/efi/sb.c file.
> > >
> > > Please take into account latest comments from Daniel A. and Glenn too.
> >
> > I don't know how to support the memtool without --enable-mm-debug at the
> > same time since the module seems to be missing then but the build system
> > still expects it on 'make install'. Unless there's an existing example of
> > how to do it I would not post with this patch.
> >
> > I can get it to create an empty module with this trick here but don't know
> > whether this helps the cause.
> >
> > GRUB_MOD_FINI (memtools)
> > {
> > #ifdef MM_DEBUG
> > grub_unregister_command (cmd_lsmem);
> > grub_unregister_command (cmd_lsfreemem);
> > grub_unregister_command (cmd_sba);
> > #else
> > (void) grub_unregister_command;
> > #endif
> > }
> >
> >
> In 1/6 we have this here. Is this sufficiently gating the usage of the
> code or do we need to use '#if defined(__powerpc__)' to only compile
> code newly added powerpc-specific code used due to this flag being
> set?
>
> +
> + if (grub_strncmp (tmp, "IBM,", 4) == 0)
> + grub_ieee1275_set_flag
> (GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY);
Good point! I think we should use "#if defined(__powerpc__)" if it is
powerpc-specific code.
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2
On Thu, Dec 01, 2022 at 08:43:56AM -0500, Stefan Berger wrote:
> On 12/1/22 00:19, Glenn Washburn wrote:
> > On Wed, 30 Nov 2022 17:42:40 -0500
> > Stefan Berger wrote:
> > > On 11/30/22 16:24, Stefan Berger wrote:
> > > > On 11/30/22 14:47, Stefan Berger wrote:
> > > > > On 11/24/22 12:56, Daniel Kiper wrote:
> > > > > > Hi,
> > > > > >
> > > > > > Adding Sudhakar and Glenn...
> > > > > >
> > > > > > On Thu, Aug 11, 2022 at 02:40:58PM -0300, Diego Domingos wrote:
> > > > > > > Hello,
> > > > > > >
> > > > > > > This is an addition to the series sent from Daniel Axtens
> > > > > > > (https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html).
> > > > > > >
> > > > > > > Patch 'ieee1275: request memory with
> > > > > > > ibm,client-architecture-support' implements vectors 1-4 of
> > > > > > > client-architecture-support negotiation However, during some
> > > > > > > tests, we found this can be a problem if:
> > > > > > >
> > > > > > > - we have more than 64 CPUs
> > > > > > > - Hardware Management Console (HMC) is configured to minimum of
> > > > > > > CPUs >64 (for example, min of 200 CPUs)
> > > > > > > - Grub needs to request memory.
> > > > > > >
> > > > > > > If vector 5 is not implemented, Power Hypervisor will consider
> > > > > > > the default value for vector 5 and 64 will bet set as the
> > > > > > > maximum number of CPUs supported by the OS, causing the machine
> > > > > > > to fail to init. Today we support 256 CPUs (max) on Power, so we
> > > > > > > need to implement vector 5 and set the MAX CPUs bits to this
> > > > > > > value.
> > > > > > >
> > > > > > > The patches 11-15 aren't merged to the grub tree yet, so I'm
> > > > > > > sending those patches again together with my patch to implement
> > > > > > > vector 5 on top of them.
> > > > > > >
> > > > > > > The patches 11-15 contains the following:
> > > > > > >
> > > > > > > Daniel Axtens (4):
> > > > > > > ieee1275: request memory with ibm,client-architecture-support
> > > > > > > ieee1275: drop len -= 1 quirk in heap_init
> > > > > > > ieee1275: support runtime memory claiming
> > > > > > > [RFC] Add memtool module with memory allocation stress-test
> > > > > > >
> > > > > > > Stefan Berger (1):
> > > > > > > ibmvtpm: Add support for trusted boot using a vTPM 2.0
> > > > > >
> > > > > > I went through all patches and cannot see major problems with
> > > > > > them. Though there are a lot of minor things which have to be
> > > > > > fixed. Sadly due to number of them I cannot simply ignore that.
> > > > > >
> > > > > > Here is the list of the issues:
> > > > > > - functions calls/sizeof(): e.g. "grub_printf()" should be
> > > > > > replaced with "grub_printf ()", add space before "(", in the
> > > > > > code; though I am OK with the former in comments and commit
> > > > > > messages,
> > > > > > - casts: e.g. "*(grub_uint32_t *)data" should be replaced with
> > > > > > "*(grub_uint32_t *) data", add space between ")" and "data",
> > > > > > - s/__attribute__((packed))/GRUB_PACKED/
> > > > > > - if you use grub_err_t type please test for GRUB_ERR_NONE
> > > > > > instead of !err or err; please do not use plain numbers, e.g. 0
> > > > > > to substitute GRUB_ERR_NONE,
> > > > > > - if you test pointers for NULL please test using NULL
> > > > > > constant instead of e.g. !ptr
> > > > > > - if you use a value often please define constant for it; good
> > > > > > candidate for such change is at least 0x3000 in the patch #3;
> > > > > > if constant definition is an overkill please comment what given
> > > > > > numbers/strings mean or at least where they come from,
> > > > > > - please do not use "//" for comments,
> > > > > > - I am OK with lines a bit longer than 80; so, please do not
> > > > > > wrap lines too early,
> > > > >
> > > > > This is a bit vague but I think I addressed them now.
> > > > >
> > > > > > - year in the copyright should be 2022.
> > > > > >
> > > > > > The GRUB coding style is described here [1] and you can find good
> > > > > > example of coding style in the grub-core/kern/efi/sb.c file.
> > > > > >
> > > > > > Please take into account latest comments from Daniel A. and Glenn
> > > > > > too.
> > > > >
> > > > > I don't know how to support the memtool without --enable-mm-debug
> > > > > at the same time since the module seems to be missing then but the
> > > > > build system still expects it on 'make install'. Unless there's an
> > > > > existing example of how to do it I would not post with this patch.
> > > > >
> > > > > I can get it to create an empty module with this trick here but
> > > > > don't know whether this helps the cause.
> > > > >
> > > > > GRUB_MOD_FINI (memtools)
> > > > > {
> > > > > #ifdef MM_DEBUG
> > > > > grub_unregister_command (cmd_lsmem);
> > > > > grub_unregister_command (cmd_lsfreemem);
> > > > > grub_unregister_command (cmd_sba);
> > > > > #else
> > > > > (void) grub_unregister_command;
> > > > > #endif
> >
Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2
On 12/1/22 09:02, Daniel Kiper wrote:
On Thu, Dec 01, 2022 at 08:43:56AM -0500, Stefan Berger wrote:
On 12/1/22 00:19, Glenn Washburn wrote:
On Wed, 30 Nov 2022 17:42:40 -0500
Stefan Berger wrote:
On 11/30/22 16:24, Stefan Berger wrote:
On 11/30/22 14:47, Stefan Berger wrote:
On 11/24/22 12:56, Daniel Kiper wrote:
Hi,
Adding Sudhakar and Glenn...
On Thu, Aug 11, 2022 at 02:40:58PM -0300, Diego Domingos wrote:
Hello,
This is an addition to the series sent from Daniel Axtens
(https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html).
Patch 'ieee1275: request memory with
ibm,client-architecture-support' implements vectors 1-4 of
client-architecture-support negotiation However, during some
tests, we found this can be a problem if:
- we have more than 64 CPUs
- Hardware Management Console (HMC) is configured to minimum of
CPUs >64 (for example, min of 200 CPUs)
- Grub needs to request memory.
If vector 5 is not implemented, Power Hypervisor will consider
the default value for vector 5 and 64 will bet set as the
maximum number of CPUs supported by the OS, causing the machine
to fail to init. Today we support 256 CPUs (max) on Power, so we
need to implement vector 5 and set the MAX CPUs bits to this
value.
The patches 11-15 aren't merged to the grub tree yet, so I'm
sending those patches again together with my patch to implement
vector 5 on top of them.
The patches 11-15 contains the following:
Daniel Axtens (4):
ieee1275: request memory with ibm,client-architecture-support
ieee1275: drop len -= 1 quirk in heap_init
ieee1275: support runtime memory claiming
[RFC] Add memtool module with memory allocation stress-test
Stefan Berger (1):
ibmvtpm: Add support for trusted boot using a vTPM 2.0
I went through all patches and cannot see major problems with
them. Though there are a lot of minor things which have to be
fixed. Sadly due to number of them I cannot simply ignore that.
Here is the list of the issues:
- functions calls/sizeof(): e.g. "grub_printf()" should be
replaced with "grub_printf ()", add space before "(", in the
code; though I am OK with the former in comments and commit
messages,
- casts: e.g. "*(grub_uint32_t *)data" should be replaced with
"*(grub_uint32_t *) data", add space between ")" and "data",
- s/__attribute__((packed))/GRUB_PACKED/
- if you use grub_err_t type please test for GRUB_ERR_NONE
instead of !err or err; please do not use plain numbers, e.g. 0
to substitute GRUB_ERR_NONE,
- if you test pointers for NULL please test using NULL
constant instead of e.g. !ptr
- if you use a value often please define constant for it; good
candidate for such change is at least 0x3000 in the patch #3;
if constant definition is an overkill please comment what given
numbers/strings mean or at least where they come from,
- please do not use "//" for comments,
- I am OK with lines a bit longer than 80; so, please do not
wrap lines too early,
This is a bit vague but I think I addressed them now.
- year in the copyright should be 2022.
The GRUB coding style is described here [1] and you can find good
example of coding style in the grub-core/kern/efi/sb.c file.
Please take into account latest comments from Daniel A. and Glenn
too.
I don't know how to support the memtool without --enable-mm-debug
at the same time since the module seems to be missing then but the
build system still expects it on 'make install'. Unless there's an
existing example of how to do it I would not post with this patch.
I can get it to create an empty module with this trick here but
don't know whether this helps the cause.
GRUB_MOD_FINI (memtools)
{
#ifdef MM_DEBUG
grub_unregister_command (cmd_lsmem);
grub_unregister_command (cmd_lsfreemem);
grub_unregister_command (cmd_sba);
#else
(void) grub_unregister_command;
#endif
}
In 1/6 we have this here. Is this sufficiently gating the usage of
the code or do we need to use '#if defined(__powerpc__)' to only
compile code newly added powerpc-specific code used due to this
flag being set?
+
+ if (grub_strncmp (tmp, "IBM,", 4) == 0)
+ grub_ieee1275_set_flag
(GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY);
And yet another question: Is __i386__ actually using
grub-core/kern/ieee1275/init.c ? I don't see it compiling this file
but there's a #ifdef __i386__ in this file.
Yes, there is a i386-ieee1275 target. It builds and the tests run
successfully, iirc.
How is this target enabled? Just configuring on an i386 host doesn't
seem to do it and I don't see an obvious configure option to build for
it, either.
./configure --target=i386 --with-platform=ieee1275 ...
I had to adjust the created symlist.h like this to make it compile at least:
//#include <../include/grub/machine/pxe.h>
//#include <../include/grub/machine/int.h>
When trying to install this on my i386 VM I get this here:
sudo grub-install /dev/vda --target=i386-ieee1275
Installing fo
Re: [PATCH v2 2/2] efi: Put Linux specific magic number in the DOS header
On Tue, Nov 29, 2022 at 06:56:16PM +0100, Ard Biesheuvel wrote: > GRUB currently relies on the magic number in the image header of ARM and > arm64 EFI kernel images to decide whether or not the image in question > is a bootable kernel. > > However, the purpose of the magic number is to identify the image as one > that implements the bare metal boot protocol, and so GRUB, which only > does EFI boot, can only boot images that could potentially be booted in > a non-EFI manner as well. > > This is problematic for the new zboot decompressor image format, as it > can only boot in EFI mode, and must therefore not use the bare metal > boot magic number in its header. > > For this reason, the strict magic number was dropped from GRUB, to > permit essentially any kind of EFI executable to be booted via the > 'linux' command, blurring the line between the linux loader and the > chainloader. > > So let's use the same field in the DOS header that RISC-V and arm64 > already use for their 'bare metal' magic numbers to store a 'generic > Linux kernel' magic number, which can be used to identify bootable > kernel images in PE format which don't necessarily implement a bare > metal boot protocol in the same binary. Note that, in the context of > EFI, the MSDOS header is only described in terms of the fields that it s/MSDOS/MS-DOS/ to be consistent with other places in the patch... > shares with the hybrid PE/COFF image format, (i.e., the magic number at > offset #0 and the PE header offset at byte offset #0x3c). Since we aim s/the magic number at offset #0/MS-DOS EXE magic number at offset #0/? "the magic number at offset #0" itself is confusing in the context of number of "magic number" phrases in the patch... :-) > for compatibility with EFI only, and not with MS-DOS or MS-Windows, we > can use the remaining space in the MS-DOS header however we want. > > Let's set the generic magic number for x86 images as well: existing > bootloaders already have their own methods to identify x86 Linux images > that can be booted in a non-EFI manner, and having the magic number in > place there will ease any future transitions in loader implementations > to merge the x86 and non-x86 EFI boot paths. > > Note that 32-bit ARM already uses the same location in the header for a > different purpose, but the ARM support is already widely implemented and > the EFI zboot decompressor is not available on ARM anyway, so we just > disregard it here. > > Cc: Huacai Chen > Cc: Atish Patra > Cc: Heinrich Schuchardt > Cc: Daniel Kiper > Cc: Leif Lindholm > Signed-off-by: Ard Biesheuvel > --- > arch/loongarch/kernel/head.S| 3 ++- > arch/x86/boot/header.S | 3 ++- > drivers/firmware/efi/libstub/zboot-header.S | 3 ++- > include/linux/pe.h | 7 +++ > 4 files changed, 13 insertions(+), 3 deletions(-) > > diff --git a/arch/loongarch/kernel/head.S b/arch/loongarch/kernel/head.S > index 84970e2666588963..caa74439700eee93 100644 > --- a/arch/loongarch/kernel/head.S > +++ b/arch/loongarch/kernel/head.S > @@ -25,7 +25,8 @@ _head: > .dword kernel_entry/* Kernel entry point */ > .dword _end - _text/* Kernel image effective size */ > .quad 0 /* Kernel image load offset from start > of RAM */ > - .org0x3c/* 0x20 ~ 0x3b reserved */ > + .org0x38/* 0x20 ~ 0x38 reserved */ > + .long LINUX_PE_MAGIC > .long pe_header - _head /* Offset to the PE header */ > > pe_header: > diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S > index f912d777013052ea..be8f78a7ee325475 100644 > --- a/arch/x86/boot/header.S > +++ b/arch/x86/boot/header.S > @@ -80,10 +80,11 @@ bs_die: > ljmp$0xf000,$0xfff0 > > #ifdef CONFIG_EFI_STUB > - .org0x3c > + .org0x38 > # > # Offset to the PE header. > # > + .long LINUX_PE_MAGIC > .long pe_header > #endif /* CONFIG_EFI_STUB */ > > diff --git a/drivers/firmware/efi/libstub/zboot-header.S > b/drivers/firmware/efi/libstub/zboot-header.S > index bc2d7750d7f14174..ec4525d40e0cf6d6 100644 > --- a/drivers/firmware/efi/libstub/zboot-header.S > +++ b/drivers/firmware/efi/libstub/zboot-header.S > @@ -20,7 +20,8 @@ __efistub_efi_zboot_header: > .long __efistub__gzdata_size - 12 // payload size > .long 0, 0// reserved > .asciz COMP_TYPE // compression > type > - .org.Ldoshdr + 0x3c > + .org.Ldoshdr + 0x38 > + .long LINUX_PE_MAGIC > .long .Lpehdr - .Ldoshdr // PE header > offset > > .Lpehdr: > diff --git a/include/linux/pe.h b/include/linux/pe.h > index 056a1762de904fc1..1db4c944efd78f51 100644 > --- a/include/linux/pe.h > +++ b/include/linux/pe.h > @@ -31,6 +31,13 @@ >
Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2
On Thu, Dec 01, 2022 at 09:22:42AM -0500, Stefan Berger wrote: > On 12/1/22 09:02, Daniel Kiper wrote: [...] > > ./configure --target=i386 --with-platform=ieee1275 ... > > I had to adjust the created symlist.h like this to make it compile at least: > > //#include <../include/grub/machine/pxe.h> > //#include <../include/grub/machine/int.h> Hmmm... Strange... It builds for me without any issues. I use latest Debian testing, gcc version 12.2.0 (Debian 12.2.0-9). It builds with gcc version 6.3.0 20170516 (Debian 6.3.0-18+deb9u1) too. What compiler do you use? > When trying to install this on my i386 VM I get this here: > > sudo grub-install /dev/vda --target=i386-ieee1275 > Installing for i386-ieee1275 platform. > grub-install: warning: cannot open directory `/usr/local/share/locale': No > such file or directory. > grub-install: warning: unknown device type vda1. > grub-install: error: ofpathname: not found. > > Would this type of target produce a grub version that works on a VM > that would otherwise work with i386-pc? Nope... > And is this still a supported target? What do you mean by "supported"? It is build tested and, AIUI, all "make check" tests pass. Though I am not sure anybody uses this kinda weird target. Well, I think I knew but I forgot... :-) Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 2/2] efi: Put Linux specific magic number in the DOS header
On Thu, 1 Dec 2022 at 15:30, Daniel Kiper wrote: > > On Tue, Nov 29, 2022 at 06:56:16PM +0100, Ard Biesheuvel wrote: > > GRUB currently relies on the magic number in the image header of ARM and > > arm64 EFI kernel images to decide whether or not the image in question > > is a bootable kernel. > > > > However, the purpose of the magic number is to identify the image as one > > that implements the bare metal boot protocol, and so GRUB, which only > > does EFI boot, can only boot images that could potentially be booted in > > a non-EFI manner as well. > > > > This is problematic for the new zboot decompressor image format, as it > > can only boot in EFI mode, and must therefore not use the bare metal > > boot magic number in its header. > > > > For this reason, the strict magic number was dropped from GRUB, to > > permit essentially any kind of EFI executable to be booted via the > > 'linux' command, blurring the line between the linux loader and the > > chainloader. > > > > So let's use the same field in the DOS header that RISC-V and arm64 > > already use for their 'bare metal' magic numbers to store a 'generic > > Linux kernel' magic number, which can be used to identify bootable > > kernel images in PE format which don't necessarily implement a bare > > metal boot protocol in the same binary. Note that, in the context of > > EFI, the MSDOS header is only described in terms of the fields that it > > s/MSDOS/MS-DOS/ to be consistent with other places in the patch... > > > shares with the hybrid PE/COFF image format, (i.e., the magic number at > > offset #0 and the PE header offset at byte offset #0x3c). Since we aim > > s/the magic number at offset #0/MS-DOS EXE magic number at offset #0/? > > "the magic number at offset #0" itself is confusing in the context of > number of "magic number" phrases in the patch... :-) > > > for compatibility with EFI only, and not with MS-DOS or MS-Windows, we > > can use the remaining space in the MS-DOS header however we want. > > > > Let's set the generic magic number for x86 images as well: existing > > bootloaders already have their own methods to identify x86 Linux images > > that can be booted in a non-EFI manner, and having the magic number in > > place there will ease any future transitions in loader implementations > > to merge the x86 and non-x86 EFI boot paths. > > > > Note that 32-bit ARM already uses the same location in the header for a > > different purpose, but the ARM support is already widely implemented and > > the EFI zboot decompressor is not available on ARM anyway, so we just > > disregard it here. > > > > Cc: Huacai Chen > > Cc: Atish Patra > > Cc: Heinrich Schuchardt > > Cc: Daniel Kiper > > Cc: Leif Lindholm > > Signed-off-by: Ard Biesheuvel > > --- > > arch/loongarch/kernel/head.S| 3 ++- > > arch/x86/boot/header.S | 3 ++- > > drivers/firmware/efi/libstub/zboot-header.S | 3 ++- > > include/linux/pe.h | 7 +++ > > 4 files changed, 13 insertions(+), 3 deletions(-) > > > > diff --git a/arch/loongarch/kernel/head.S b/arch/loongarch/kernel/head.S > > index 84970e2666588963..caa74439700eee93 100644 > > --- a/arch/loongarch/kernel/head.S > > +++ b/arch/loongarch/kernel/head.S > > @@ -25,7 +25,8 @@ _head: > > .dword kernel_entry/* Kernel entry point */ > > .dword _end - _text/* Kernel image effective size */ > > .quad 0 /* Kernel image load offset from > > start of RAM */ > > - .org0x3c/* 0x20 ~ 0x3b reserved */ > > + .org0x38/* 0x20 ~ 0x38 reserved */ > > + .long LINUX_PE_MAGIC > > .long pe_header - _head /* Offset to the PE header */ > > > > pe_header: > > diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S > > index f912d777013052ea..be8f78a7ee325475 100644 > > --- a/arch/x86/boot/header.S > > +++ b/arch/x86/boot/header.S > > @@ -80,10 +80,11 @@ bs_die: > > ljmp$0xf000,$0xfff0 > > > > #ifdef CONFIG_EFI_STUB > > - .org0x3c > > + .org0x38 > > # > > # Offset to the PE header. > > # > > + .long LINUX_PE_MAGIC > > .long pe_header > > #endif /* CONFIG_EFI_STUB */ > > > > diff --git a/drivers/firmware/efi/libstub/zboot-header.S > > b/drivers/firmware/efi/libstub/zboot-header.S > > index bc2d7750d7f14174..ec4525d40e0cf6d6 100644 > > --- a/drivers/firmware/efi/libstub/zboot-header.S > > +++ b/drivers/firmware/efi/libstub/zboot-header.S > > @@ -20,7 +20,8 @@ __efistub_efi_zboot_header: > > .long __efistub__gzdata_size - 12 // payload > > size > > .long 0, 0// reserved > > .asciz COMP_TYPE // > > compression type > > - .org.Ldoshdr + 0x3c > > + .org.Ldoshdr + 0x38 > > + .long LINUX_PE_MAGIC > > .long
Re: [PATCH v2 2/2] efi: Put Linux specific magic number in the DOS header
On Thu, Dec 01, 2022 at 03:48:12PM +0100, Ard Biesheuvel wrote: > On Thu, 1 Dec 2022 at 15:30, Daniel Kiper wrote: > > On Tue, Nov 29, 2022 at 06:56:16PM +0100, Ard Biesheuvel wrote: > > > GRUB currently relies on the magic number in the image header of ARM and > > > arm64 EFI kernel images to decide whether or not the image in question > > > is a bootable kernel. > > > > > > However, the purpose of the magic number is to identify the image as one > > > that implements the bare metal boot protocol, and so GRUB, which only > > > does EFI boot, can only boot images that could potentially be booted in > > > a non-EFI manner as well. > > > > > > This is problematic for the new zboot decompressor image format, as it > > > can only boot in EFI mode, and must therefore not use the bare metal > > > boot magic number in its header. > > > > > > For this reason, the strict magic number was dropped from GRUB, to > > > permit essentially any kind of EFI executable to be booted via the > > > 'linux' command, blurring the line between the linux loader and the > > > chainloader. > > > > > > So let's use the same field in the DOS header that RISC-V and arm64 > > > already use for their 'bare metal' magic numbers to store a 'generic > > > Linux kernel' magic number, which can be used to identify bootable > > > kernel images in PE format which don't necessarily implement a bare > > > metal boot protocol in the same binary. Note that, in the context of > > > EFI, the MSDOS header is only described in terms of the fields that it > > > > s/MSDOS/MS-DOS/ to be consistent with other places in the patch... > > > > > shares with the hybrid PE/COFF image format, (i.e., the magic number at > > > offset #0 and the PE header offset at byte offset #0x3c). Since we aim > > > > s/the magic number at offset #0/MS-DOS EXE magic number at offset #0/? > > > > "the magic number at offset #0" itself is confusing in the context of > > number of "magic number" phrases in the patch... :-) > > > > > for compatibility with EFI only, and not with MS-DOS or MS-Windows, we > > > can use the remaining space in the MS-DOS header however we want. > > > > > > Let's set the generic magic number for x86 images as well: existing > > > bootloaders already have their own methods to identify x86 Linux images > > > that can be booted in a non-EFI manner, and having the magic number in > > > place there will ease any future transitions in loader implementations > > > to merge the x86 and non-x86 EFI boot paths. > > > > > > Note that 32-bit ARM already uses the same location in the header for a > > > different purpose, but the ARM support is already widely implemented and > > > the EFI zboot decompressor is not available on ARM anyway, so we just > > > disregard it here. > > > > > > Cc: Huacai Chen > > > Cc: Atish Patra > > > Cc: Heinrich Schuchardt > > > Cc: Daniel Kiper > > > Cc: Leif Lindholm > > > Signed-off-by: Ard Biesheuvel > > > --- > > > arch/loongarch/kernel/head.S| 3 ++- > > > arch/x86/boot/header.S | 3 ++- > > > drivers/firmware/efi/libstub/zboot-header.S | 3 ++- > > > include/linux/pe.h | 7 +++ > > > 4 files changed, 13 insertions(+), 3 deletions(-) > > > > > > diff --git a/arch/loongarch/kernel/head.S b/arch/loongarch/kernel/head.S > > > index 84970e2666588963..caa74439700eee93 100644 > > > --- a/arch/loongarch/kernel/head.S > > > +++ b/arch/loongarch/kernel/head.S > > > @@ -25,7 +25,8 @@ _head: > > > .dword kernel_entry/* Kernel entry point */ > > > .dword _end - _text/* Kernel image effective size */ > > > .quad 0 /* Kernel image load offset from > > > start of RAM */ > > > - .org0x3c/* 0x20 ~ 0x3b reserved */ > > > + .org0x38/* 0x20 ~ 0x38 reserved */ > > > + .long LINUX_PE_MAGIC > > > .long pe_header - _head /* Offset to the PE header */ > > > > > > pe_header: > > > diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S > > > index f912d777013052ea..be8f78a7ee325475 100644 > > > --- a/arch/x86/boot/header.S > > > +++ b/arch/x86/boot/header.S > > > @@ -80,10 +80,11 @@ bs_die: > > > ljmp$0xf000,$0xfff0 > > > > > > #ifdef CONFIG_EFI_STUB > > > - .org0x3c > > > + .org0x38 > > > # > > > # Offset to the PE header. > > > # > > > + .long LINUX_PE_MAGIC > > > .long pe_header > > > #endif /* CONFIG_EFI_STUB */ > > > > > > diff --git a/drivers/firmware/efi/libstub/zboot-header.S > > > b/drivers/firmware/efi/libstub/zboot-header.S > > > index bc2d7750d7f14174..ec4525d40e0cf6d6 100644 > > > --- a/drivers/firmware/efi/libstub/zboot-header.S > > > +++ b/drivers/firmware/efi/libstub/zboot-header.S > > > @@ -20,7 +20,8 @@ __efistub_efi_zboot_header: > > > .long __efistub__gzdata_size - 12 // payload > > > size > > > .long 0, 0
Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2
On 12/1/22 09:47, Daniel Kiper wrote:
On Thu, Dec 01, 2022 at 09:22:42AM -0500, Stefan Berger wrote:
On 12/1/22 09:02, Daniel Kiper wrote:
[...]
./configure --target=i386 --with-platform=ieee1275 ...
I had to adjust the created symlist.h like this to make it compile at least:
//#include <../include/grub/machine/pxe.h>
//#include <../include/grub/machine/int.h>
Hmmm... Strange... It builds for me without any issues. I use latest
Debian testing, gcc version 12.2.0 (Debian 12.2.0-9). It builds with
gcc version 6.3.0 20170516 (Debian 6.3.0-18+deb9u1) too. What compiler
do you use?
$ cat /etc/debian_version
11.5
$ gcc --version
gcc (Debian 10.2.1-6) 10.2.1 20210110
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
When trying to install this on my i386 VM I get this here:
sudo grub-install /dev/vda --target=i386-ieee1275
Installing for i386-ieee1275 platform.
grub-install: warning: cannot open directory `/usr/local/share/locale': No such
file or directory.
grub-install: warning: unknown device type vda1.
grub-install: error: ofpathname: not found.
Would this type of target produce a grub version that works on a VM
that would otherwise work with i386-pc?
Nope...
Well, not good...
And is this still a supported target?
What do you mean by "supported"? It is build tested and, AIUI, all "make check"
tests pass. Though I am not sure anybody uses this kinda weird target. Well,
I think I knew but I forgot... :-)
The problem is I don't know much about the target other than ieee1275 being
OpenFirmare or so. So we (I) could easily break someone's favorite target here
...
The following function from this series would probably run on i386-ieee1275 as
well since these nodes are probably so fundamental to OpenFirmware that there's
no point in surrounding it with #ifdef __powerpc__ ?
static grub_err_t
grub_ieee1275_total_mem (grub_uint64_t *total)
{
grub_ieee1275_phandle_t root;
grub_ieee1275_phandle_t memory;
grub_uint32_t reg[4];
grub_ssize_t reg_size;
grub_uint32_t address_cells = 1;
grub_uint32_t size_cells = 1;
grub_uint64_t size;
/* If we fail to get to the end, report 0. */
*total = 0;
/* Determine the format of each entry in `reg'. */
if (grub_ieee1275_finddevice ("/", &root))
return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "couldn't find / node");
if (grub_ieee1275_get_integer_property (root, "#address-cells",
&address_cells,
sizeof (address_cells), 0))
return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "couldn't examine
#address-cells");
if (grub_ieee1275_get_integer_property (root, "#size-cells", &size_cells,
sizeof (size_cells), 0))
return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "couldn't examine #size-cells");
I added the if's here just to be 'safe'.
In the end i386-ieee1275 shouldn't hold up progress on powerpc ieee1275...
Stefan
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2
On Thu, Dec 01, 2022 at 09:58:45AM -0500, Stefan Berger wrote:
> On 12/1/22 09:47, Daniel Kiper wrote:
> > On Thu, Dec 01, 2022 at 09:22:42AM -0500, Stefan Berger wrote:
> > > On 12/1/22 09:02, Daniel Kiper wrote:
> >
> > [...]
> >
> > > > ./configure --target=i386 --with-platform=ieee1275 ...
> > >
> > > I had to adjust the created symlist.h like this to make it compile at
> > > least:
> > >
> > > //#include <../include/grub/machine/pxe.h>
> > > //#include <../include/grub/machine/int.h>
> >
> > Hmmm... Strange... It builds for me without any issues. I use latest
> > Debian testing, gcc version 12.2.0 (Debian 12.2.0-9). It builds with
> > gcc version 6.3.0 20170516 (Debian 6.3.0-18+deb9u1) too. What compiler
> > do you use?
>
> $ cat /etc/debian_version
> 11.5
> $ gcc --version
> gcc (Debian 10.2.1-6) 10.2.1 20210110
> Copyright (C) 2020 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Hmmm... It should work...
> > > When trying to install this on my i386 VM I get this here:
> > >
> > > sudo grub-install /dev/vda --target=i386-ieee1275
> > > Installing for i386-ieee1275 platform.
> > > grub-install: warning: cannot open directory `/usr/local/share/locale':
> > > No such file or directory.
> > > grub-install: warning: unknown device type vda1.
> > > grub-install: error: ofpathname: not found.
> > >
> > > Would this type of target produce a grub version that works on a VM
> > > that would otherwise work with i386-pc?
> >
> > Nope...
>
> Well, not good...
>
> >
> > > And is this still a supported target?
> >
> > What do you mean by "supported"? It is build tested and, AIUI, all "make
> > check"
> > tests pass. Though I am not sure anybody uses this kinda weird target. Well,
> > I think I knew but I forgot... :-)
>
> The problem is I don't know much about the target other than ieee1275
> being OpenFirmare or so. So we (I) could easily break someone's
> favorite target here ...
:-) I am happy you care about that. I think Glenn could tell you how to
run "make check" for i386-ieee1275 target. Glenn?
And I think you can find some hints how to run "make check" in the
INSTALL file too...
> The following function from this series would probably run on
> i386-ieee1275 as well since these nodes are probably so fundamental to
> OpenFirmware that there's no point in surrounding it with #ifdef
> __powerpc__ ?
Yeah, I think you are right...
> static grub_err_t
> grub_ieee1275_total_mem (grub_uint64_t *total)
> {
> grub_ieee1275_phandle_t root;
> grub_ieee1275_phandle_t memory;
> grub_uint32_t reg[4];
> grub_ssize_t reg_size;
> grub_uint32_t address_cells = 1;
> grub_uint32_t size_cells = 1;
> grub_uint64_t size;
>
> /* If we fail to get to the end, report 0. */
> *total = 0;
>
> /* Determine the format of each entry in `reg'. */
> if (grub_ieee1275_finddevice ("/", &root))
> return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "couldn't find / node");
> if (grub_ieee1275_get_integer_property (root, "#address-cells",
> &address_cells,
> sizeof (address_cells), 0))
> return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "couldn't examine
> #address-cells");
> if (grub_ieee1275_get_integer_property (root, "#size-cells", &size_cells,
> sizeof (size_cells), 0))
> return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "couldn't examine
> #size-cells");
>
> I added the if's here just to be 'safe'.
This should be enough. Though please double check if other parts of the
code fail safely if grub_ieee1275_total_mem() returns an error.
> In the end i386-ieee1275 shouldn't hold up progress on powerpc ieee1275...
Yeah, I concur...
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2
On 12/1/22 10:51, Daniel Kiper wrote:
On Thu, Dec 01, 2022 at 09:58:45AM -0500, Stefan Berger wrote:
On 12/1/22 09:47, Daniel Kiper wrote:
On Thu, Dec 01, 2022 at 09:22:42AM -0500, Stefan Berger wrote:
On 12/1/22 09:02, Daniel Kiper wrote:
[...]
./configure --target=i386 --with-platform=ieee1275 ...
I had to adjust the created symlist.h like this to make it compile at least:
//#include <../include/grub/machine/pxe.h>
//#include <../include/grub/machine/int.h>
Hmmm... Strange... It builds for me without any issues. I use latest
Debian testing, gcc version 12.2.0 (Debian 12.2.0-9). It builds with
gcc version 6.3.0 20170516 (Debian 6.3.0-18+deb9u1) too. What compiler
do you use?
$ cat /etc/debian_version
11.5
$ gcc --version
gcc (Debian 10.2.1-6) 10.2.1 20210110
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Hmmm... It should work...
Not typically using Debian but Debian 11.5 was the i386 iso I could find. Well,
it works fine for i386-pc.
When trying to install this on my i386 VM I get this here:
sudo grub-install /dev/vda --target=i386-ieee1275
Installing for i386-ieee1275 platform.
grub-install: warning: cannot open directory `/usr/local/share/locale': No such
file or directory.
grub-install: warning: unknown device type vda1.
grub-install: error: ofpathname: not found.
Would this type of target produce a grub version that works on a VM
that would otherwise work with i386-pc?
Nope...
Well, not good...
And is this still a supported target?
What do you mean by "supported"? It is build tested and, AIUI, all "make check"
tests pass. Though I am not sure anybody uses this kinda weird target. Well,
I think I knew but I forgot... :-)
The problem is I don't know much about the target other than ieee1275
being OpenFirmare or so. So we (I) could easily break someone's
favorite target here ...
:-) I am happy you care about that. I think Glenn could tell you how to
The more esoteric the target the more ... worrisome ? :-)
run "make check" for i386-ieee1275 target. Glenn?
I just ran 'make check' (as user) for i386-pc on current master (and with the
patches applied). The fallout was this:
FAIL: squashfs_test[blkid missing]
FAIL: help_testno particular helpful message
FAIL: grub_func_test only this type of error:
tests/video_checksum.c:checksum:615: assert failed: 0 Checksum
cmdline_cat_2560x1440xrgba:44 failed: 0x62031fea vs 0x8071678a
Now on the same i386 machine with i386-ieee1275 (master): [after installing
some more packages to be able to run the tests I can now build it fine]
FAIL: squashfs_test
FAIL: ahci_test
FAIL: grub_script_eval
FAIL: pata_test
FAIL: example_grub_script_test
FAIL: grub_script_leading_whitespace
FAIL: ehci_test
FAIL: ohci_test
FAIL: grub_script_test
FAIL: grub_script_echo1
FAIL: uhci_test
FAIL: grub_script_echo_keywords
FAIL: grub_script_vars1
FAIL: grub_script_for1
FAIL: grub_script_while1
FAIL: grub_script_if
FAIL: grub_script_comments
FAIL: grub_script_break
FAIL: grub_script_return
FAIL: grub_script_shift
FAIL: grub_cmd_regexp
FAIL: grub_script_functions
FAIL: grub_script_blockarg
FAIL: grub_script_continue
FAIL: grub_script_setparams
FAIL: grub_cmd_set_date
FAIL: grub_cmd_date
FAIL: grub_func_test
FAIL: grub_cmd_sleep
FAIL: grub_script_not
FAIL: hddboot_test
FAIL: fddboot_test
FAIL: xzcompress_test
FAIL: help_test
FAIL: test_unset
FAIL: grub_cmd_echo
FAIL: grub_script_gettext
FAIL: grub_script_escape_comma
FAIL: test_sha512sum
FAIL: grub_script_strcmp
FAIL: grub_cmd_tr
FAIL: file_filter_test
FAIL: gzcompress_test
FAIL: lzocompress_test
So I take this as the baseline for me now -- interpretation is: i386-ieee1275
is borked.
And I think you can find some hints how to run "make check" in the
INSTALL file too...
I wished there was a line apt-get to install them all. Something like this
seems a good base but still need qemu:
sudo apt install dosfstools gzip lzop xz-utils attr cpio g++ gawk parted recode
tar util-linux btrfs-progs dosfstools e2fsprogs f2fs-tools genromfs hfsprogs
jfsutils nilfs-tools ntfs-3g reiserfsprogs squashfs-tools reiserfsprogs
udftools xfsprogs zfs-fuse exfat-fuse
The following function from this series would probably run on
i386-ieee1275 as well since these nodes are probably so fundamental to
OpenFirmware that there's no point in surrounding it with #ifdef
__powerpc__ ?
Yeah, I think you are right...
static grub_err_t
grub_ieee1275_total_mem (grub_uint64_t *total)
{
grub_ieee1275_phandle_t root;
grub_ieee1275_phandle_t memory;
grub_uint32_t reg[4];
grub_ssize_t reg_size;
grub_uint32_t address_cells = 1;
grub_uint32_t size_cells = 1;
grub_uint64_t size;
/* If we fail to get to the end, report 0. */
*total = 0;
/* Determine the format of each entry in `reg'. */
if (grub_ieee1275_finddevice
Re: [PATCH 0/6] Dynamic allocation of memory regions and IBM vTPM v2
On Thu, Dec 01, 2022 at 11:44:02AM -0500, Stefan Berger wrote: > On 12/1/22 10:51, Daniel Kiper wrote: > > On Thu, Dec 01, 2022 at 09:58:45AM -0500, Stefan Berger wrote: > > > On 12/1/22 09:47, Daniel Kiper wrote: > > > > On Thu, Dec 01, 2022 at 09:22:42AM -0500, Stefan Berger wrote: > > > > > On 12/1/22 09:02, Daniel Kiper wrote: > > > > > > > > [...] > > > > > > > > > > ./configure --target=i386 --with-platform=ieee1275 ... > > > > > > > > > > I had to adjust the created symlist.h like this to make it compile at > > > > > least: > > > > > > > > > > //#include <../include/grub/machine/pxe.h> > > > > > //#include <../include/grub/machine/int.h> > > > > > > > > Hmmm... Strange... It builds for me without any issues. I use latest > > > > Debian testing, gcc version 12.2.0 (Debian 12.2.0-9). It builds with > > > > gcc version 6.3.0 20170516 (Debian 6.3.0-18+deb9u1) too. What compiler > > > > do you use? > > > > > > $ cat /etc/debian_version > > > 11.5 > > > $ gcc --version > > > gcc (Debian 10.2.1-6) 10.2.1 20210110 > > > Copyright (C) 2020 Free Software Foundation, Inc. > > > This is free software; see the source for copying conditions. There is NO > > > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR > > > PURPOSE. > > > > Hmmm... It should work... > > Not typically using Debian but Debian 11.5 was the i386 iso I could find. > Well, it works fine for i386-pc. > > > > > > When trying to install this on my i386 VM I get this here: > > > > > > > > > > sudo grub-install /dev/vda --target=i386-ieee1275 > > > > > Installing for i386-ieee1275 platform. > > > > > grub-install: warning: cannot open directory > > > > > `/usr/local/share/locale': No such file or directory. > > > > > grub-install: warning: unknown device type vda1. > > > > > grub-install: error: ofpathname: not found. > > > > > > > > > > Would this type of target produce a grub version that works on a VM > > > > > that would otherwise work with i386-pc? > > > > > > > > Nope... > > > > > > Well, not good... > > > > > > > > > > > > And is this still a supported target? > > > > > > > > What do you mean by "supported"? It is build tested and, AIUI, all > > > > "make check" > > > > tests pass. Though I am not sure anybody uses this kinda weird target. > > > > Well, > > > > I think I knew but I forgot... :-) > > > > > > The problem is I don't know much about the target other than ieee1275 > > > being OpenFirmare or so. So we (I) could easily break someone's > > > favorite target here ... > > > > :-) I am happy you care about that. I think Glenn could tell you how to > > The more esoteric the target the more ... worrisome ? :-) :-) > > run "make check" for i386-ieee1275 target. Glenn? > > I just ran 'make check' (as user) for i386-pc on current master (and with the > patches applied). The fallout was this: > > FAIL: squashfs_test[blkid missing] > FAIL: help_testno particular helpful message > FAIL: grub_func_test only this type of error: > tests/video_checksum.c:checksum:615: assert failed: 0 Checksum > cmdline_cat_2560x1440xrgba:44 failed: 0x62031fea vs 0x8071678a > > Now on the same i386 machine with i386-ieee1275 (master): [after installing > some more packages to be able to run the tests I can now build it fine] > > FAIL: squashfs_test > FAIL: ahci_test > FAIL: grub_script_eval > FAIL: pata_test > FAIL: example_grub_script_test > FAIL: grub_script_leading_whitespace > FAIL: ehci_test > FAIL: ohci_test > FAIL: grub_script_test > FAIL: grub_script_echo1 > FAIL: uhci_test > FAIL: grub_script_echo_keywords > FAIL: grub_script_vars1 > FAIL: grub_script_for1 > FAIL: grub_script_while1 > FAIL: grub_script_if > FAIL: grub_script_comments > FAIL: grub_script_break > FAIL: grub_script_return > FAIL: grub_script_shift > FAIL: grub_cmd_regexp > FAIL: grub_script_functions > FAIL: grub_script_blockarg > FAIL: grub_script_continue > FAIL: grub_script_setparams > FAIL: grub_cmd_set_date > FAIL: grub_cmd_date > FAIL: grub_func_test > FAIL: grub_cmd_sleep > FAIL: grub_script_not > FAIL: hddboot_test > FAIL: fddboot_test > FAIL: xzcompress_test > FAIL: help_test > FAIL: test_unset > FAIL: grub_cmd_echo > FAIL: grub_script_gettext > FAIL: grub_script_escape_comma > FAIL: test_sha512sum > FAIL: grub_script_strcmp > FAIL: grub_cmd_tr > FAIL: file_filter_test > FAIL: gzcompress_test > FAIL: lzocompress_test > > So I take this as the baseline for me now -- interpretation is: i386-ieee1275 > is borked. Glenn? Any idea? > > And I think you can find some hints how to run "make check" in the > > INSTALL file too... > > I wished there was a line apt-get to install them all. Something like this > seems a good base but still need qemu: > > sudo apt install dosfstools gzip lzop xz-utils attr cpio g++ gawk parted > recode tar util-linux btrfs-progs dosfstools e2fsprogs f2fs-tools genromfs > hfsprogs jfsutils nilfs-tools ntfs-3g reiserfsprogs squashfs-tools > reiserfsprogs udftools xfsprogs zfs-fuse exfat-fuse Feel free to add it
Re: [PATCH V2 2/4] bash-completion:fix shellcheck SC2207-Warning
On Wed, Nov 30, 2022 at 04:30:58PM +0800, t.feng wrote:
> SC2207 (warning): Prefer mapfile or read -a to split
> command output (or quote to avoid splitting).
>
> In grub-completion.bash.in line 56:
> COMPREPLY=($(compgen -P "${2-}" -W "${1-}" -S "${4-}" --
> "$cur"))
>^-- SC2207 (warning)
>
> In grub-completion.bash.in line 119:
> COMPREPLY=( $(compgen \
> ^-- SC2207 (warning)
>
> In grub-completion.bash.in line 128:
> COMPREPLY=( $( compgen -f -X '!*/*.mod' -- "${grub_dir}/$cur" | {
> ^-- SC2207 (warning)
>
> ref:https://github.com/koalaman/shellcheck/wiki/SC2207
>
> Signed-off-by: "t.feng"
> ---
> .../bash-completion.d/grub-completion.bash.in | 34 ---
> 1 file changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/util/bash-completion.d/grub-completion.bash.in
> b/util/bash-completion.d/grub-completion.bash.in
> index 93d143480..4749cbc64 100644
> --- a/util/bash-completion.d/grub-completion.bash.in
> +++ b/util/bash-completion.d/grub-completion.bash.in
> @@ -53,7 +53,10 @@ __grubcomp () {
> ;;
> *)
> local IFS=' '$'\t'$'\n'
> -COMPREPLY=($(compgen -P "${2-}" -W "${1-}" -S "${4-}" -- "$cur"))
> +COMPREPLY=()
> +while read -r line; do
I think the "line" variable should be local. Same for other variables below...
And I think it would be nice if you add a patch which marks all existing
functions variables as local.
> +COMPREPLY+=("${line}")
> +done < <(compgen -P "${2-}" -W "${1-}" -S "${4-}" -- "$cur")
> ;;
> esac
> }
> @@ -116,24 +119,29 @@ __grub_list_menuentries () {
>
> if [ -f "$config_file" ];then
> local IFS=$'\n'
> -COMPREPLY=( $(compgen \
> --W "$( awk -F "[\"']" '/menuentry/ { print $2 }' $config_file )"
> \
> --- "$cur" )) #'# Help emacs syntax highlighting
> +COMPREPLY=()
> +while read -r line; do
> +COMPREPLY+=("${line}")
> +done < <(compgen \
> +-W "$( awk -F "[\"']" '/menuentry/ { print $2 }'
> $config_file )" \
> +-- "$cur" ) #'# Help emacs syntax highlighting
> fi
> }
>
> __grub_list_modules () {
> local grub_dir=$(__grub_dir)
> local IFS=$'\n'
> -COMPREPLY=( $( compgen -f -X '!*/*.mod' -- "${grub_dir}/$cur" | {
> - while read -r tmp; do
> - [ -n "$tmp" ] && {
> - tmp=${tmp##*/}
> - printf '%s\n' ${tmp%.mod}
> - }
> - done
> - }
> -))
> +COMPREPLY=()
> +while read -r line; do
> +COMPREPLY+=("${line}")
> +done < <(compgen -f -X '!*/*.mod' -- "${grub_dir}/$cur" | {
> +while read -r tmp; do
> +[ -n "$tmp" ] && {
> +tmp=${tmp##*/}
> +printf '%s\n' ${tmp%.mod}
> +}
> +done
> +})
> }
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] util/grub.d/30_uefi-firmware.in: Re-arrange if conditions
On Tue, Nov 29, 2022 at 10:13:27PM +, Dimitri John Ledkov wrote: > Only perform call to fwsetup if one is on EFI platform. On all other > platorms fwsetup command does not exists, and thus returns 0 and a > useless uefi-firmware menuentry gets generated. > > Signed-off-by: Dimitri John Ledkov Reviewed-by: Daniel Kiper Thank you! Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 2/2] efi: Put Linux specific magic number in the DOS header
On Thu, Dec 01, 2022 at 03:48:12PM +0100, Ard Biesheuvel wrote: > On Thu, 1 Dec 2022 at 15:30, Daniel Kiper wrote: > > > > On Tue, Nov 29, 2022 at 06:56:16PM +0100, Ard Biesheuvel wrote: > > > GRUB currently relies on the magic number in the image header of ARM and > > > arm64 EFI kernel images to decide whether or not the image in question > > > is a bootable kernel. > > > > > > However, the purpose of the magic number is to identify the image as one > > > that implements the bare metal boot protocol, and so GRUB, which only > > > does EFI boot, can only boot images that could potentially be booted in > > > a non-EFI manner as well. > > > > > > This is problematic for the new zboot decompressor image format, as it > > > can only boot in EFI mode, and must therefore not use the bare metal > > > boot magic number in its header. > > > > > > For this reason, the strict magic number was dropped from GRUB, to > > > permit essentially any kind of EFI executable to be booted via the > > > 'linux' command, blurring the line between the linux loader and the > > > chainloader. > > > > > > So let's use the same field in the DOS header that RISC-V and arm64 > > > already use for their 'bare metal' magic numbers to store a 'generic > > > Linux kernel' magic number, which can be used to identify bootable > > > kernel images in PE format which don't necessarily implement a bare > > > metal boot protocol in the same binary. Note that, in the context of > > > EFI, the MSDOS header is only described in terms of the fields that it > > > > s/MSDOS/MS-DOS/ to be consistent with other places in the patch... > > > > > shares with the hybrid PE/COFF image format, (i.e., the magic number at > > > offset #0 and the PE header offset at byte offset #0x3c). Since we aim > > > > s/the magic number at offset #0/MS-DOS EXE magic number at offset #0/? > > > > "the magic number at offset #0" itself is confusing in the context of > > number of "magic number" phrases in the patch... :-) > > > > > for compatibility with EFI only, and not with MS-DOS or MS-Windows, we > > > can use the remaining space in the MS-DOS header however we want. > > > > > > Let's set the generic magic number for x86 images as well: existing > > > bootloaders already have their own methods to identify x86 Linux images > > > that can be booted in a non-EFI manner, and having the magic number in > > > place there will ease any future transitions in loader implementations > > > to merge the x86 and non-x86 EFI boot paths. > > > > > > Note that 32-bit ARM already uses the same location in the header for a > > > different purpose, but the ARM support is already widely implemented and > > > the EFI zboot decompressor is not available on ARM anyway, so we just > > > disregard it here. > > > > > > Cc: Huacai Chen > > > Cc: Atish Patra > > > Cc: Heinrich Schuchardt > > > Cc: Daniel Kiper > > > Cc: Leif Lindholm > > > Signed-off-by: Ard Biesheuvel > > > --- > > > arch/loongarch/kernel/head.S| 3 ++- > > > arch/x86/boot/header.S | 3 ++- > > > drivers/firmware/efi/libstub/zboot-header.S | 3 ++- > > > include/linux/pe.h | 7 +++ > > > 4 files changed, 13 insertions(+), 3 deletions(-) > > > > > > diff --git a/arch/loongarch/kernel/head.S b/arch/loongarch/kernel/head.S > > > index 84970e2666588963..caa74439700eee93 100644 > > > --- a/arch/loongarch/kernel/head.S > > > +++ b/arch/loongarch/kernel/head.S > > > @@ -25,7 +25,8 @@ _head: > > > .dword kernel_entry/* Kernel entry point */ > > > .dword _end - _text/* Kernel image effective size */ > > > .quad 0 /* Kernel image load offset from > > > start of RAM */ > > > - .org0x3c/* 0x20 ~ 0x3b reserved */ > > > + .org0x38/* 0x20 ~ 0x38 reserved */ Sadly one more thing... :-( s/0x20 ~ 0x38 reserved/0x20 ~ 0x37 reserved/ Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH V2 4/4] bash-completion:disable shellcheck SC2120-Warning
On Wed, Nov 30, 2022 at 04:31:00PM +0800, t.feng wrote:
> In grub-completion.bash.in line 63:
> __grub_get_options_from_help () {
> ^-- SC2120 (warning)
Again, what the warning means?
> SC2120: the current code meets the exception and does not need to be
> modified. So we disable it.
>
> ref:https://github.com/koalaman/shellcheck/wiki/SC2120
>
> Signed-off-by: "t.feng"
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v8 1/1] plainmount: Support plain encryption mode
On Wed, Nov 30, 2022 at 11:58:33PM -0600, Glenn Washburn wrote:
> On Tue, 29 Nov 2022 18:13:11 +0100
> Daniel Kiper wrote:
>
> > First of all, sorry for late reply. I hope I will be able to review
> > next version of this patch much faster.
> >
> > On Sat, Oct 29, 2022 at 05:40:42PM +, Maxim Fomin wrote:
> > > From 2b1d2deb3f2416cbc3e7d25cbc4141a3078eaf68 Mon Sep 17 00:00:00
> > > 2001 From: Maxim Fomin
> > > Date: Sat, 29 Oct 2022 18:18:58 +0100
> > > Subject: [PATCH v8 1/1] plainmount: Support plain encryption mode
> > >
> > > This patch adds support for plain encryption mode (plain dm-crypt)
> > > via new module/command named 'plainmount'.
> > >
> > > Signed-off-by: Maxim Fomin
> >
> > Please put "---" behind SOB and before "Difference with..." next
> > time...
>
> [...]
>
> > > +
> > > +/* Configure cryptodisk uuid */
> > > +static void plainmount_set_uuid (grub_cryptodisk_t dev, const char
> > > *user_uuid) +{
> > > + grub_size_t pos = 0;
> > > +
> > > + /* Size of user_uuid is checked in main func */
> > > + if (user_uuid != NULL)
> > > + grub_memcpy (dev->uuid, user_uuid, grub_strlen (user_uuid));
> >
> > I think grub_strcpy() instead of grub_memcpy() would be more natural
> > in string contexts. And then you do not need grub_strlen().
> >
> > Is it safe to assume here that dev->uuid has been zeroed earlier?
>
> Yes, its zalloc'd in grub_cmd_plainmount below.
OK, cool!
[...]
> > > + /* Check uuid length */
> > > + if (state[OPTION_UUID].set && grub_strlen
> > > (state[OPTION_UUID].arg) >
> > > + sizeof (PLAINMOUNT_DEFAULT_UUID))
> > > +return grub_error (GRUB_ERR_BAD_ARGUMENT,
> > > +N_("specified UUID exceeds maximum size"));
> >
> > What will happen if somebody passes shorter, probably non-valid, UUID?
> > I think we should check UUID is not shorter than something too.
>
> Afaik, we don't really care what the UUID string is, just that its
> unique. It doesn't have to be a real UUID either. A minimum length
> check of 1 might be good, otherwise '--uuid ""' will create an empty
> uuid, but its not a big deal either. We might think about a character
> validation check so that characters like ')' can't be in the string.
> None of this will cause real problems as far as I can tell, but some
> features may not work right. For example, having ')' in the uuid will
> make it so that you can't access the crypto device via the
> '(cruuid/)' syntax. I also don't mind not doing the validation
> and letting the user shoot themselves in the foot if they so choose.
I think we should check UUID size and probably it should not be smaller
than 1 and larger than "sizeof(PLAINMOUNT_DEFAULT_UUID) - 1". I do not
think we should check validity/content of the UUID.
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH V2 1/4] bash-completion:fix shellcheck error
On Wed, Nov 30, 2022 at 04:30:57PM +0800, t.feng wrote:
> SC2070 (error): -n doesn't work with unquoted arguments.
> Quote or use [[ ]].
> In grub-completion.bash.in line 130:
> [ -n $tmp ] && {
> ^--^ SC2070 (error)
>
> ref:https://github.com/koalaman/shellcheck/wiki/SC2070
>
> Signed-off-by: "t.feng"
Reviewed-by: Daniel Kiper
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH V2 3/4] bash-completion:fix shellcheck SC2155-Warning
On Wed, Nov 30, 2022 at 04:30:59PM +0800, t.feng wrote: > In grub-completion.bash.in line 115: > local config_file=$(__grub_dir)/grub.cfg > ^-^ SC2155 (warning) > > In grub-completion.bash.in line 126: > local grub_dir=$(__grub_dir) > ^--^ SC2155 (warning) Please add at least one sentence of description to the commit message why this patch is needed. Otherwise almost everybody has to take a look at the link below what the warning means. Though link may stay as is... > ref:https://github.com/koalaman/shellcheck/wiki/SC2155 Please drop "ref:" or replace it with "More: ". Same applies to the other patches. > Signed-off-by: "t.feng" Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v8 1/1] plainmount: Support plain encryption mode
On Sat, 29 Oct 2022 17:40:42 +
Maxim Fomin wrote:
> From 2b1d2deb3f2416cbc3e7d25cbc4141a3078eaf68 Mon Sep 17 00:00:00 2001
> From: Maxim Fomin
> Date: Sat, 29 Oct 2022 18:18:58 +0100
> Subject: [PATCH v8 1/1] plainmount: Support plain encryption mode
>
> This patch adds support for plain encryption mode (plain dm-crypt) via
> new module/command named 'plainmount'.
>
> Signed-off-by: Maxim Fomin
>
> Difference with v7:
Daniel pointed this out, but this isn't a well formed patch. I do very
much appreciate you adding the differences in as it made it easier to
look at this. And my suggestion was to use --interdiff or --range-diff
to the format-patch command, which would properly format things. It
looks like you just copy pasted the output of "git diff v7 v8".
I'm now compiling this patch and found a few compile issues below.
You're compile testing this right?
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 377969984..34ca6b4f1 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -5138,13 +5138,13 @@ to generate password hashes. @xref{Security}.
>
> Setup access to the encrypted device in plain mode. Offset of the
> encrypted -data at the device is specified in terms of 512 byte
> sectors with the blocklist +data at the device is specified in terms
> of 512 byte sectors using the blocklist syntax and loopback device.
> The following example shows how to specify 1MiB offset:
>
> @example
> loopback node (hd0,gpt1)2048+
> -plainmount node
> +plainmount node @var{...}
> @end example
>
> The @command{plainmount} command can be used to open LUKS encrypted
> volume @@ -5155,13 +5155,14 @@ The keyfile path parameter has higher
> priority than the secret passphrase parameter and is specified with
> the option @option{-d}. Password data obtained from keyfiles is not
> hashed and is used directly as a cipher key. An optional offset of
> password data in the keyfile can be specified with the option
> -@option{-O} or directly with the option @option{-d} and GRUB
> blocklist syntax. +@option{-O} or directly with the option
> @option{-d} and GRUB blocklist syntax, +if the keyfile data can be
> accessed from a device and is 512 byte aligned. The following example
> shows both methods to specify password data in the keyfile at offset
> 1MiB:
>
> @example
> -plainmount -d (hd0,gpt1)2048+
> -plainmount -d (hd0,gpt1)+ -O 1048576
> +plainmount -d (hd0,gpt1)2048+ @var{...}
> +plainmount -d (hd0,gpt1)+ -O 1048576 @var{...}
> @end example
>
> If no keyfile is specified then the password is set to the string
> specified diff --git a/grub-core/disk/plainmount.c
> b/grub-core/disk/plainmount.c index 656c5d09f..85ada25bc 100644
> --- a/grub-core/disk/plainmount.c
> +++ b/grub-core/disk/plainmount.c
> @@ -146,8 +146,12 @@ plainmount_configure_password (grub_cryptodisk_t
> dev, const char *hash, dev->hash = grub_crypto_lookup_md_by_name
> (hash); len = dev->hash->mdlen;
>
> - alloc_size = password_size >= key_size ? password_size : key_size;
> - p = grub_zalloc (alloc_size + (alloc_size / len));
> + alloc_size = grub_max (password_size, key_size);
> + /*
> + * Allocate buffer for the password and for an added prefix
> character
> + * for each hash round ('alloc_size' may not be a multiple of
> 'len').
> + */
> + p = grub_zalloc (alloc_size + (alloc_size / len) + 1);
>derived_hash = grub_zalloc (GRUB_CRYPTODISK_MAX_KEYLEN * 2);
>if (p == NULL || derived_hash == NULL)
> {
> @@ -170,9 +174,10 @@ plainmount_configure_password (grub_cryptodisk_t
> dev, const char *hash, if (len > size)
> len = size;
>
> - grub_crypto_hash (dev->hash, dh, p, password_size);
> + grub_crypto_hash (dev->hash, dh, p, password_size + round);
> }
> - grub_memcpy (key_data, derived_hash, GRUB_CRYPTODISK_MAX_KEYLEN *
> 2);
> + grub_memcpy (key_data, derived_hash, key_size);
> +
> exit:
>grub_free (p);
>grub_free (derived_hash);
> ---
> docs/grub.texi | 81 +++
> grub-core/Makefile.core.def | 5 +
> grub-core/disk/plainmount.c | 462
> 3 files changed, 548
> insertions(+) create mode 100644 grub-core/disk/plainmount.c
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 2d6cd8358..34ca6b4f1 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -4271,6 +4271,7 @@ you forget a command, you can run the command
> @command{help}
> * parttool::Modify partition table entries
> * password::Set a clear-text password
> * password_pbkdf2:: Set a hashed password
> +* plainmount:: Open device encrypted in plain mode
> * play::Play a tune
> * probe:: Retrieve device info
> * rdmsr:: Read values from model-specific
> registers @@ -4558,6 +4559,14 @@ function is supported, as Argon2 is
> not yet supported.
> Also, note that, unlike filesystem UUIDs, UUIDs for encrypted
> devices m
[PATCH v2 0/6] Dynamic allocation of memory regions and IBM vTPM v2
Hello, This is an addition to the series sent from Daniel Axtens (https://lists.gnu.org/archive/html/grub-devel/2022-04/msg00064.html). Patch 'ieee1275: request memory with ibm,client-architecture-support' implements vectors 1-4 of client-architecture-support negotiation However, during some tests, we found this can be a problem if: - we have more than 64 CPUs - Hardware Management Console (HMC) is configured to minimum of CPUs >64 (for example, min of 200 CPUs) - Grub needs to request memory. If vector 5 is not implemented, Power Hypervisor will consider the default value for vector 5 and 64 will bet set as the maximum number of CPUs supported by the OS, causing the machine to fail to init. Today we support 256 CPUs (max) on Power, so we need to implement vector 5 and set the MAX CPUs bits to this value. Stefan v2: - Followed Daniel K.'s list of suggestions checking the list more than twice and adjusted formatting, line breaks, etc. in all patches - Conditional compilation of ppc64-specific code using #ifdef __powerpc__ - Fixing of installation of memtool; adding of warning message for __powerpc__ since it requires a reboot after running memory stress test - Tested it on i386-pc and PowerKVM Daniel Axtens (4): ieee1275: request memory with ibm, client-architecture-support ieee1275: drop len -= 1 quirk in heap_init ieee1275: support runtime memory claiming Add memtool module with memory allocation stress-test Diego Domingos (1): ieee1275: implement vec5 for cas negotiation Stefan Berger (1): ibmvtpm: Add support for trusted boot using a vTPM 2.0 configure.ac | 1 + docs/grub-dev.texi| 7 +- docs/grub.texi| 3 +- grub-core/Makefile.core.def | 13 + grub-core/commands/ieee1275/ibmvtpm.c | 151 + grub-core/commands/memtools.c | 160 ++ grub-core/kern/ieee1275/cmain.c | 3 + grub-core/kern/ieee1275/init.c| 443 +- grub-core/kern/mm.c | 4 + include/grub/ieee1275/ieee1275.h | 11 + 10 files changed, 779 insertions(+), 17 deletions(-) create mode 100644 grub-core/commands/ieee1275/ibmvtpm.c create mode 100644 grub-core/commands/memtools.c -- 2.25.1 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2 3/6] ieee1275: support runtime memory claiming
From: Daniel Axtens On powerpc-ieee1275, we are running out of memory trying to verify anything. This is because: - we have to load an entire file into memory to verify it. This is difficult to change with appended signatures. - We only have 32MB of heap. - Distro kernels are now often around 30MB. So we want to be able to claim more memory from OpenFirmware for our heap at runtime. There are some complications: - The grub mm code isn't the only thing that will make claims on memory from OpenFirmware: * PFW/SLOF will have claimed some for their own use. * The ieee1275 loader will try to find other bits of memory that we haven't claimed to place the kernel and initrd when we go to boot. * Once we load Linux, it will also try to claim memory. It claims memory without any reference to /memory/available, it just starts at min(top of RMO, 768MB) and works down. So we need to avoid this area. See arch/powerpc/kernel/prom_init.c as of v5.11. - The smallest amount of memory a ppc64 KVM guest can have is 256MB. It doesn't work with distro kernels but can work with custom kernels. We should maintain support for that. (ppc32 can boot with even less, and we shouldn't break that either.) - Even if a VM has more memory, the memory OpenFirmware makes available as Real Memory Area can be restricted. Even with our CAS work, an LPAR on a PowerVM box is likely to have only 512MB available to OpenFirmware even if it has many gigabytes of memory allocated. What should we do? We don't know in advance how big the kernel and initrd are going to be, which makes figuring out how much memory we can take a bit tricky. To figure out how much memory we should leave unused, I looked at: - an Ubuntu 20.04.1 ppc64le pseries KVM guest: vmlinux: ~30MB initrd: ~50MB - a RHEL8.2 ppc64le pseries KVM guest: vmlinux: ~30MB initrd: ~30MB So to give us a little wriggle room, I think we want to leave at least 128MB for the loader to put vmlinux and initrd in memory and leave Linux with space to satisfy its early allocations. Allow other space to be allocated at runtime. Tested-by: Stefan Berger Signed-off-by: Daniel Axtens --- docs/grub-dev.texi | 7 +- grub-core/kern/ieee1275/init.c | 268 ++--- 2 files changed, 255 insertions(+), 20 deletions(-) diff --git a/docs/grub-dev.texi b/docs/grub-dev.texi index f76fc658b..31eb99ea2 100644 --- a/docs/grub-dev.texi +++ b/docs/grub-dev.texi @@ -1059,7 +1059,10 @@ space is limited to 4GiB. GRUB allocates pages from EFI for its heap, at most 1.6 GiB. On i386-ieee1275 and powerpc-ieee1275 GRUB uses same stack as IEEE1275. -It allocates at most 32MiB for its heap. + +On i386-ieee1275 and powerpc-ieee1275, GRUB will allocate 32MiB for its heap on +startup. It may allocate more at runtime, as long as at least 128MiB remain free +in OpenFirmware. On sparc64-ieee1275 stack is 256KiB and heap is 2MiB. @@ -1087,7 +1090,7 @@ In short: @item i386-qemu @tab 60 KiB @tab < 4 GiB @item *-efi @tab ? @tab < 1.6 GiB @item i386-ieee1275 @tab ? @tab < 32 MiB -@item powerpc-ieee1275@tab ? @tab < 32 MiB +@item powerpc-ieee1275@tab ? @tab available memory - 128MiB @item sparc64-ieee1275@tab 256KiB @tab 2 MiB @item arm-uboot @tab 256KiB @tab 2 MiB @item mips(el)-qemu_mips @tab 2MiB@tab 253 MiB diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c index bb234b268..e5fdc1fd2 100644 --- a/grub-core/kern/ieee1275/init.c +++ b/grub-core/kern/ieee1275/init.c @@ -45,13 +45,26 @@ #include #endif -/* The maximum heap size we're going to claim */ +/* The maximum heap size we're going to claim at boot. Not used by sparc. */ #ifdef __i386__ #define HEAP_MAX_SIZE (unsigned long) (64 * 1024 * 1024) -#else +#else /* __powerpc__ */ #define HEAP_MAX_SIZE (unsigned long) (32 * 1024 * 1024) #endif +/* RMO max. address at 768 MB */ +#define RMO_ADDR_MAX (grub_uint64_t) (768 * 1024 * 1024) + +/* + * The amount of OF space we will not claim here so as to leave space for + * the loader and linux to service early allocations. + * + * In 2021, Daniel Axtens claims that we should leave at least 128MB to + * ensure we can load a stock kernel and initrd on a pseries guest with + * a 512MB real memory area under PowerVM. + */ +#define RUNTIME_MIN_SPACE (128UL * 1024 * 1024) + extern char _start[]; extern char _end[]; @@ -145,16 +158,52 @@ grub_claim_heap (void) + GRUB_KERNEL_MACHINE_STACK_SIZE), 0x20); } #else -/* Helper for grub_claim_heap. */ +/* Helpers for mm on powerpc. */ + +/* + * How much memory does OF believe exists in total? + * + * This isn't necessarily the true total. It can be the total memory + * accessible in real mode for a pseries guest, for example. + */ +st
[PATCH v2 4/6] ieee1275: implement vec5 for cas negotiation
From: Diego Domingos
As a legacy support, if the vector 5 is not implemented, Power Hypervisor will
consider the max CPUs as 64 instead 256 currently supported during
client-architecture-support negotiation.
This patch implements the vector 5 and set the MAX CPUs to 256 while setting the
others values to 0 (default).
Signed-off-by: Diego Domingos
Acked-by: Daniel Axtens
Signed-off-by: Stefan Berger
---
grub-core/kern/ieee1275/init.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
index e5fdc1fd2..870f89a54 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -498,6 +498,19 @@ struct option_vector2
grub_uint8_t max_pft_size;
} GRUB_PACKED;
+struct option_vector5
+{
+ grub_uint8_t byte1;
+ grub_uint8_t byte2;
+ grub_uint8_t byte3;
+ grub_uint8_t cmo;
+ grub_uint8_t associativity;
+ grub_uint8_t bin_opts;
+ grub_uint8_t micro_checkpoint;
+ grub_uint8_t reserved0;
+ grub_uint32_t max_cpus;
+} GRUB_PACKED;
+
struct pvr_entry
{
grub_uint32_t mask;
@@ -519,6 +532,8 @@ struct cas_vector
grub_uint16_t vec3;
grub_uint8_t vec4_size;
grub_uint16_t vec4;
+ grub_uint8_t vec5_size;
+ struct option_vector5 vec5;
} GRUB_PACKED;
/*
@@ -543,7 +558,7 @@ grub_ieee1275_ibm_cas (void)
struct cas_vector vector =
{
.pvr_list = { { 0x, 0x } }, /* any processor */
-.num_vecs = 4 - 1,
+.num_vecs = 5 - 1,
.vec1_size = 0,
.vec1 = 0x80, /* ignore */
.vec2_size = 1 + sizeof (struct option_vector2) - 2,
@@ -554,6 +569,10 @@ grub_ieee1275_ibm_cas (void)
.vec3 = 0x00e0, /* ask for FP + VMX + DFP but don't halt if unsatisfied */
.vec4_size = 2 - 1,
.vec4 = 0x0001, /* set required minimum capacity % to the lowest value */
+.vec5_size = 1 + sizeof (struct option_vector5) - 2,
+.vec5 = {
+ 0, 0, 0, 0, 0, 0, 0, 0, 256
+}
};
INIT_IEEE1275_COMMON (&args.common, "call-method", 3, 2);
--
2.25.1
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2 5/6] Add memtool module with memory allocation stress-test
From: Daniel Axtens
When working on memory, it's nice to be able to test your work.
Add a memtest module. When compiled with --enable-mm-debug, it exposes
3 commands:
* lsmem - print all allocations and free space in all regions
* lsfreemem - print free space in all regions
* stress_big_allocs - stress test large allocations:
- how much memory can we allocate in one chunk?
- how many 1MB chunks can we allocate?
- check that gap-filling works with a 1MB aligned 900kB alloc + a
100kB alloc.
Signed-off-by: Daniel Axtens
Signed-off-by: Stefan Berger
---
configure.ac | 1 +
grub-core/Makefile.core.def | 6 ++
grub-core/commands/memtools.c | 160 ++
grub-core/kern/mm.c | 4 +
4 files changed, 171 insertions(+)
create mode 100644 grub-core/commands/memtools.c
diff --git a/configure.ac b/configure.ac
index 93626b798..ca42ff8f7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1520,6 +1520,7 @@ else
MM_DEBUG=0
fi
AC_SUBST([MM_DEBUG])
+AM_CONDITIONAL([COND_MM_DEBUG], [test x$MM_DEBUG = x1])
AC_ARG_ENABLE([cache-stats],
AS_HELP_STRING([--enable-cache-stats],
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 95942fc8c..0ec95997d 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -2542,3 +2542,9 @@ module = {
common = commands/i386/wrmsr.c;
enable = x86;
};
+
+module = {
+ name = memtools;
+ common = commands/memtools.c;
+ condition = COND_MM_DEBUG;
+};
diff --git a/grub-core/commands/memtools.c b/grub-core/commands/memtools.c
new file mode 100644
index 0..3471e5941
--- /dev/null
+++ b/grub-core/commands/memtools.c
@@ -0,0 +1,160 @@
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2022 Free Software Foundation, Inc.
+ * Copyright (C) 2022 IBM Corporation
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see
[PATCH v2 6/6] ibmvtpm: Add support for trusted boot using a vTPM 2.0
Add support for trusted boot using a vTPM 2.0 on the IBM IEEE1275
PowerPC platform. With this patch grub now measures text and binary data
into the TPM's PCRs 8 and 9 in the same way as the x86_64 platform
does.
This patch requires Daniel Axtens's patches for claiming more memory.
For vTPM support to work on PowerVM, system driver levels 1010.30
or 1020.00 are required.
Note: Previous versions of firmware levels with the 2hash-ext-log
API call have a bug that, once this API call is invoked, has the
effect of disabling the vTPM driver under Linux causing an error
message to be displayed in the Linux kernel log. Those users will
have to update their machines to the firmware levels mentioned
above.
Cc: Eric Snowberg
Signed-off-by: Stefan Berger
Signed-off-by: Daniel Axtens
---
docs/grub.texi| 3 +-
grub-core/Makefile.core.def | 7 ++
grub-core/commands/ieee1275/ibmvtpm.c | 151 ++
include/grub/ieee1275/ieee1275.h | 3 +
4 files changed, 163 insertions(+), 1 deletion(-)
create mode 100644 grub-core/commands/ieee1275/ibmvtpm.c
diff --git a/docs/grub.texi b/docs/grub.texi
index 50c811a88..580f2a995 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -6413,7 +6413,8 @@ tpm module is loaded. As such it is recommended that the
tpm module be built
into @file{core.img} in order to avoid a potential gap in measurement between
@file{core.img} being loaded and the tpm module being loaded.
-Measured boot is currently only supported on EFI platforms.
+Measured boot is currently only supported on EFI and IBM IEEE1275 PowerPC
+platforms.
@node Lockdown
@section Lockdown when booting on a secure setup
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 0ec95997d..ef6964b91 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -1133,6 +1133,13 @@ module = {
enable = powerpc_ieee1275;
};
+module = {
+ name = tpm;
+ common = commands/tpm.c;
+ ieee1275 = commands/ieee1275/ibmvtpm.c;
+ enable = powerpc_ieee1275;
+};
+
module = {
name = terminal;
common = commands/terminal.c;
diff --git a/grub-core/commands/ieee1275/ibmvtpm.c
b/grub-core/commands/ieee1275/ibmvtpm.c
new file mode 100644
index 0..78e002c47
--- /dev/null
+++ b/grub-core/commands/ieee1275/ibmvtpm.c
@@ -0,0 +1,151 @@
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2022 Free Software Foundation, Inc.
+ * Copyright (C) 2022 IBM Corporation
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see
[PATCH v2 2/6] ieee1275: drop len -= 1 quirk in heap_init
From: Daniel Axtens
This was apparently 'required by some firmware': commit dc9468500919
("2007-02-12 Hollis Blanchard ").
It's not clear what firmware that was, and what platform from 14 years ago
which exhibited the bug then is still both in use and buggy now.
It doesn't cause issues on qemu (mac99 or pseries) or under PFW for Power8.
I don't have access to old Mac hardware, but if anyone feels especially
strongly we can put it under some feature flag. I really want to disable
it under pseries because it will mess with region merging.
Signed-off-by: Daniel Axtens
---
grub-core/kern/ieee1275/init.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
index 0bc571e3e..bb234b268 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -166,7 +166,6 @@ heap_init (grub_uint64_t addr, grub_uint64_t len,
grub_memory_type_t type,
addr = 0x18;
}
}
- len -= 1; /* Required for some firmware. */
/* Never exceed HEAP_MAX_SIZE */
if (*total + len > HEAP_MAX_SIZE)
--
2.25.1
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2 1/6] ieee1275: request memory with ibm, client-architecture-support
From: Daniel Axtens
On PowerVM, the first time we boot a Linux partition, we may only get
256MB of real memory area, even if the partition has more memory.
This isn't enough to reliably verify a kernel. Fortunately, the Power
Architecture Platform Reference (PAPR) defines a method we can call to ask
for more memory: the broad and powerful ibm,client-architecture-support
(CAS) method.
CAS can do an enormous amount of things on a PAPR platform: as well as
asking for memory, you can set the supported processor level, the interrupt
controller, hash vs radix mmu, and so on.
If:
- we are running under what we think is PowerVM (compatible property of /
begins with "IBM"), and
- the full amount of RMA is less than 512MB (as determined by the reg
property of /memory)
then call CAS as follows: (refer to the Linux on Power Architecture
Reference, LoPAR, which is public, at B.5.2.3):
- Use the "any" PVR value and supply 2 option vectors.
- Set option vector 1 (PowerPC Server Processor Architecture Level)
to "ignore".
- Set option vector 2 with default or Linux-like options, including a
min-rma-size of 512MB.
- Set option vector 3 to request Floating Point, VMX and Decimal Floating
point, but don't abort the boot if we can't get them.
- Set option vector 4 to request a minimum VP percentage to 1%, which is
what Linux requests, and is below the default of 10%. Without this,
some systems with very large or very small configurations fail to boot.
This will cause a CAS reboot and the partition will restart with 512MB
of RMA. Importantly, grub will notice the 512MB and not call CAS again.
Notes about the choices of parameters:
- A partition can be configured with only 256MB of memory, which would
mean this request couldn't be satisfied, but PFW refuses to load with
only 256MB of memory, so it's a bit moot. SLOF will run fine with 256MB,
but we will never call CAS under qemu/SLOF because /compatible won't
begin with "IBM".)
- unspecified CAS vectors take on default values. Some of these values
might restrict the ability of certain hardware configurations to boot.
This is why we need to specify the VP percentage in vector 4, which is
in turn why we need to specify vector 3.
Finally, we should have enough memory to verify a kernel, and we will
reach Linux. One of the first things Linux does while still running under
OpenFirmware is to call CAS with a much fuller set of options (including
asking for 512MB of memory). Linux includes a much more restrictive set of
PVR values and processor support levels, and this CAS invocation will likely
induce another reboot. On this reboot grub will again notice the higher RMA,
and not call CAS. We will get to Linux again, Linux will call CAS again, but
because the values are now set for Linux this will not induce another CAS
reboot and we will finally boot all the way to userspace.
On all subsequent boots, everything will be configured with 512MB of RMA,
so there will be no further CAS reboots from grub. (phyp is super sticky
with the RMA size - it persists even on cold boots. So if you've ever booted
Linux in a partition, you'll probably never have grub call CAS. It'll only
ever fire the first time a partition loads grub, or if you deliberately lower
the amount of memory your partition has below 512MB.)
Signed-off-by: Daniel Axtens
Signed-off-by: Stefan Berger
---
grub-core/kern/ieee1275/cmain.c | 3 +
grub-core/kern/ieee1275/init.c | 165 +++
include/grub/ieee1275/ieee1275.h | 8 ++
3 files changed, 176 insertions(+)
diff --git a/grub-core/kern/ieee1275/cmain.c b/grub-core/kern/ieee1275/cmain.c
index 4442b6a83..b707798ec 100644
--- a/grub-core/kern/ieee1275/cmain.c
+++ b/grub-core/kern/ieee1275/cmain.c
@@ -123,6 +123,9 @@ grub_ieee1275_find_options (void)
break;
}
}
+
+ if (grub_strncmp (tmp, "IBM,", 4) == 0)
+ grub_ieee1275_set_flag (GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY);
}
if (is_smartfirmware)
diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
index 2adf4fdfc..0bc571e3e 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -200,11 +200,176 @@ heap_init (grub_uint64_t addr, grub_uint64_t len,
grub_memory_type_t type,
return 0;
}
+/*
+ * How much memory does OF believe it has? (regardless of whether
+ * it's accessible or not)
+ */
+static grub_err_t
+grub_ieee1275_total_mem (grub_uint64_t *total)
+{
+ grub_ieee1275_phandle_t root;
+ grub_ieee1275_phandle_t memory;
+ grub_uint32_t reg[4];
+ grub_ssize_t reg_size;
+ grub_uint32_t address_cells = 1;
+ grub_uint32_t size_cells = 1;
+ grub_uint64_t size;
+
+ /* If we fail to get to the end, report 0. */
+ *total = 0;
+
+ /* Determine the format of each entry in `reg'. */
+ if (grub_ieee1275_finddevice ("/", &root))
+return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "couldn't find / node");
+ if (gr
[PATCH 1/7] acpi: Export a generic grub_acpi_find_table
From: Matthias Lange
And convert grub_acpi_find_fadt to use it
Signed-off-by: Benjamin Herrenschmidt
---
grub-core/kern/acpi.c | 43 +--
include/grub/acpi.h | 3 +++
2 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/grub-core/kern/acpi.c b/grub-core/kern/acpi.c
index 70898ddcd..5d5b54b72 100644
--- a/grub-core/kern/acpi.c
+++ b/grub-core/kern/acpi.c
@@ -85,35 +85,42 @@ grub_acpi_xsdt_find_table (struct grub_acpi_table_header
*xsdt, const char *sig)
return 0;
}
-struct grub_acpi_fadt *
-grub_acpi_find_fadt (void)
+void *
+grub_acpi_find_table (const char *sig)
{
- struct grub_acpi_fadt *fadt = 0;
+ struct grub_acpi_fadt *r = 0;
struct grub_acpi_rsdp_v10 *rsdpv1;
struct grub_acpi_rsdp_v20 *rsdpv2;
+
rsdpv1 = grub_machine_acpi_get_rsdpv1 ();
if (rsdpv1)
-fadt = grub_acpi_rsdt_find_table ((struct grub_acpi_table_header *)
- (grub_addr_t) rsdpv1->rsdt_addr,
- GRUB_ACPI_FADT_SIGNATURE);
- if (fadt)
-return fadt;
+r = grub_acpi_rsdt_find_table ((struct grub_acpi_table_header *)
+ (grub_addr_t) rsdpv1->rsdt_addr,
+ sig);
+ if (r)
+return r;
rsdpv2 = grub_machine_acpi_get_rsdpv2 ();
if (rsdpv2)
-fadt = grub_acpi_rsdt_find_table ((struct grub_acpi_table_header *)
- (grub_addr_t) rsdpv2->rsdpv1.rsdt_addr,
- GRUB_ACPI_FADT_SIGNATURE);
- if (fadt)
-return fadt;
+r = grub_acpi_rsdt_find_table ((struct grub_acpi_table_header *)
+ (grub_addr_t) rsdpv2->rsdpv1.rsdt_addr,
+ sig);
+ if (r)
+return r;
if (rsdpv2
#if GRUB_CPU_SIZEOF_VOID_P != 8
&& !(rsdpv2->xsdt_addr >> 32)
#endif
)
-fadt = grub_acpi_xsdt_find_table ((struct grub_acpi_table_header *)
- (grub_addr_t) rsdpv2->xsdt_addr,
- GRUB_ACPI_FADT_SIGNATURE);
- if (fadt)
-return fadt;
+r = grub_acpi_xsdt_find_table ((struct grub_acpi_table_header *)
+ (grub_addr_t) rsdpv2->xsdt_addr,
+ sig);
+ if (r)
+return r;
return 0;
}
+
+struct grub_acpi_fadt *
+grub_acpi_find_fadt (void)
+{
+ return grub_acpi_find_table (GRUB_ACPI_FADT_SIGNATURE);
+}
diff --git a/include/grub/acpi.h b/include/grub/acpi.h
index 84f49487d..8c126b2b9 100644
--- a/include/grub/acpi.h
+++ b/include/grub/acpi.h
@@ -244,4 +244,7 @@ enum
struct grub_acpi_fadt *
EXPORT_FUNC(grub_acpi_find_fadt) (void);
+void *
+EXPORT_FUNC(grub_acpi_find_table) (const char *sig);
+
#endif /* ! GRUB_ACPI_HEADER */
--
2.34.1
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 3/7] ns8250: Add base support for MMIO UARTs
From: Benjamin Herrenschmidt
This adds the ability for the driver to access UARTs via MMIO instead
of PIO selectively at runtime, and exposes a new function to add an
MMIO port.
Signed-off-by: Benjamin Herrenschmidt
---
grub-core/term/ns8250.c | 78 -
grub-core/term/serial.c | 8 +++--
include/grub/serial.h | 11 +-
3 files changed, 78 insertions(+), 19 deletions(-)
diff --git a/grub-core/term/ns8250.c b/grub-core/term/ns8250.c
index 83b25990c..b640508d0 100644
--- a/grub-core/term/ns8250.c
+++ b/grub-core/term/ns8250.c
@@ -43,6 +43,24 @@ static int dead_ports = 0;
#define DEFAULT_BASE_CLOCK 115200
#endif
+static inline unsigned char
+ns8250_reg_read (struct grub_serial_port *port, grub_addr_t reg)
+{
+ asm volatile("" : : : "memory");
+ if (port->mmio)
+return *((volatile unsigned char *)(port->mmio_base + reg));
+ return grub_inb (port->port + reg);
+}
+
+static inline void
+ns8250_reg_write (struct grub_serial_port *port, unsigned char value,
grub_addr_t reg)
+{
+ asm volatile("" : : : "memory");
+ if (port->mmio)
+*((volatile char *)(port->mmio_base + reg)) = value;
+ else
+grub_outb (value, port->port + reg);
+}
/* Convert speed to divisor. */
static unsigned short
@@ -93,43 +111,42 @@ do_real_config (struct grub_serial_port *port)
divisor = serial_get_divisor (port, &port->config);
/* Turn off the interrupt. */
- grub_outb (0, port->port + UART_IER);
+ ns8250_reg_write (port, 0, UART_IER);
/* Set DLAB. */
- grub_outb (UART_DLAB, port->port + UART_LCR);
+ ns8250_reg_write (port, UART_DLAB, UART_LCR);
- /* Set the baud rate. */
- grub_outb (divisor & 0xFF, port->port + UART_DLL);
- grub_outb (divisor >> 8, port->port + UART_DLH);
+ ns8250_reg_write (port, divisor & 0xFF, UART_DLL);
+ ns8250_reg_write (port, divisor >> 8, UART_DLH);
/* Set the line status. */
status |= (parities[port->config.parity]
| (port->config.word_len - 5)
| stop_bits[port->config.stop_bits]);
- grub_outb (status, port->port + UART_LCR);
+ ns8250_reg_write (port, status, UART_LCR);
if (port->config.rtscts)
{
/* Enable the FIFO. */
- grub_outb (UART_ENABLE_FIFO_TRIGGER1, port->port + UART_FCR);
+ ns8250_reg_write (port, UART_ENABLE_FIFO_TRIGGER1, UART_FCR);
/* Turn on DTR and RTS. */
- grub_outb (UART_ENABLE_DTRRTS, port->port + UART_MCR);
+ ns8250_reg_write (port, UART_ENABLE_DTRRTS, UART_MCR);
}
else
{
/* Enable the FIFO. */
- grub_outb (UART_ENABLE_FIFO_TRIGGER14, port->port + UART_FCR);
+ ns8250_reg_write (port, UART_ENABLE_FIFO_TRIGGER14, UART_FCR);
/* Turn on DTR, RTS, and OUT2. */
- grub_outb (UART_ENABLE_DTRRTS | UART_ENABLE_OUT2, port->port + UART_MCR);
+ ns8250_reg_write (port, UART_ENABLE_DTRRTS | UART_ENABLE_OUT2, UART_MCR);
}
/* Drain the input buffer. */
endtime = grub_get_time_ms () + 1000;
- while (grub_inb (port->port + UART_LSR) & UART_DATA_READY)
+ while (ns8250_reg_read (port, UART_LSR) & UART_DATA_READY)
{
- grub_inb (port->port + UART_RX);
+ ns8250_reg_read (port, UART_RX);
if (grub_get_time_ms () > endtime)
{
port->broken = 1;
@@ -145,8 +162,8 @@ static int
serial_hw_fetch (struct grub_serial_port *port)
{
do_real_config (port);
- if (grub_inb (port->port + UART_LSR) & UART_DATA_READY)
-return grub_inb (port->port + UART_RX);
+ if (ns8250_reg_read (port, UART_LSR) & UART_DATA_READY)
+return ns8250_reg_read (port, UART_RX);
return -1;
}
@@ -166,7 +183,7 @@ serial_hw_put (struct grub_serial_port *port, const int c)
else
endtime = grub_get_time_ms () + 200;
/* Wait until the transmitter holding register is empty. */
- while ((grub_inb (port->port + UART_LSR) & UART_EMPTY_TRANSMITTER) == 0)
+ while ((ns8250_reg_read (port, UART_LSR) & UART_EMPTY_TRANSMITTER) == 0)
{
if (grub_get_time_ms () > endtime)
{
@@ -179,7 +196,7 @@ serial_hw_put (struct grub_serial_port *port, const int c)
if (port->broken)
port->broken--;
- grub_outb (c, port->port + UART_TX);
+ ns8250_reg_write (port, c, UART_TX);
}
/* Initialize a serial device. PORT is the port number for a serial device.
@@ -262,6 +279,7 @@ grub_ns8250_init (void)
com_ports[i].name = com_names[i];
com_ports[i].driver = &grub_ns8250_driver;
com_ports[i].port = serial_hw_io_addr[i];
+ com_ports[i].mmio = 0;
err = grub_serial_config_defaults (&com_ports[i]);
if (err)
grub_print_error ();
@@ -316,8 +334,36 @@ grub_serial_ns8250_add_port (grub_port_t port)
}
p->driver = &grub_ns8250_driver;
grub_serial_config_defaults (p);
+ p->mmio = 0;
p->port = port;
grub_serial_register (p);
return p->name;
}
+
+char *
+grub_serial_ns8250_add_mmio(grub_addr_t addr)
+{
+ struct grub_serial_port *p;
+ unsigned i;
+ for (i = 0; i < GRUB_SERIAL
[RESEND PATCH 0/7] serial: Add MMIO & SPCR support
This series adds support for MMIO serial ports and auto-configuration via ACPI SPCR. This is necessary for the serial port to work on AWS EC2 "metal" x86 systems. An MMIO serial port can be specified explicitely using the "mmio," prefix for the --port argument to the 'serial' command, the register access size can also be specified via a suffix, and when 'serial' is used without arguments, it will try to use an ACPI SPCR entry (if any) to add the default port and configure it appropriately (speed, parity etc...) This was tested using SPCR on an actual c5.metal instance, and using explicit instanciation via serial -p mmio on a modified qemu hacked to create MMIO PCI serial ports. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 2/7] acpi: Add SPCR and generic address definitions
From: Benjamin Herrenschmidt
Signed-off-by: Benjamin Herrenschmidt
---
include/grub/acpi.h | 51 +
1 file changed, 51 insertions(+)
diff --git a/include/grub/acpi.h b/include/grub/acpi.h
index 8c126b2b9..17aadb802 100644
--- a/include/grub/acpi.h
+++ b/include/grub/acpi.h
@@ -179,6 +179,57 @@ enum
GRUB_ACPI_MADT_ENTRY_SAPIC_FLAGS_ENABLED = 1
};
+struct grub_acpi_genaddr {
+ grub_uint8_t space_id;
+#define GRUB_ACPI_GENADDR_MEM_SPACE 0x00
+#define GRUB_ACPI_GENADDR_IO_SPACE 0x01
+ grub_uint8_t bit_width;
+ grub_uint8_t bit_offset;
+ grub_uint8_t access_size;
+#define GRUB_ACPI_GENADDR_SIZE_LGCY 0x00
+#define GRUB_ACPI_GENADDR_SIZE_BYTE 0x01
+#define GRUB_ACPI_GENADDR_SIZE_WORD 0x02
+#define GRUB_ACPI_GENADDR_SIZE_DWORD 0x03
+#define GRUB_ACPI_GENADDR_SIZE_QWORD 0x04
+ grub_uint64_t addr;
+} GRUB_PACKED;
+
+#define GRUB_ACPI_SPCR_SIGNATURE "SPCR"
+
+struct grub_acpi_spcr {
+ struct grub_acpi_table_header hdr;
+ grub_uint8_t intf_type;
+#define GRUB_ACPI_SPCR_INTF_TYPE_16550 0x00
+#define GRUB_ACPI_SPCR_INTF_TYPE_16550X 0x01
+ grub_uint8_t reserved_0[3];
+ struct grub_acpi_genaddr base_addr;
+ grub_uint8_t interrupt_type;
+ grub_uint8_t irq;
+ grub_uint32_t gsi;
+ grub_uint8_t baud_rate;
+#define GRUB_ACPI_SPCR_BAUD_CURRENT 0x00
+#define GRUB_ACPI_SPCR_BAUD_9600 0x03
+#define GRUB_ACPI_SPCR_BAUD_192000x04
+#define GRUB_ACPI_SPCR_BAUD_576000x06
+#define GRUB_ACPI_SPCR_BAUD_115200 0x07
+ grub_uint8_t parity;
+ grub_uint8_t stop_bits;
+ grub_uint8_t flow_control;
+#define GRUB_ACPI_SPCR_FC_DCD_TX (1 << 0)
+#define GRUB_ACPI_SPCR_FC_RTSCTS (1 << 1)
+#define GRUB_ACPI_SPCR_FC_XONXOFF(1 << 2)
+ grub_uint8_t terminal_type;
+ grub_uint8_t language;
+ grub_uint16_tpci_device_id;
+ grub_uint16_t pci_vendor_id;
+ grub_uint8_t pci_bus;
+ grub_uint8_t pci_device;
+ grub_uint8_t pci_function;
+ grub_uint32_t pci_flags;
+ grub_uint8_t pci_segment;
+ grub_uint32_t reserved_1;
+} GRUB_PACKED;
+
#ifndef GRUB_DSDT_TEST
struct grub_acpi_rsdp_v10 *grub_acpi_get_rsdpv1 (void);
struct grub_acpi_rsdp_v20 *grub_acpi_get_rsdpv2 (void);
--
2.34.1
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 6/7] ns8250: Support more MMIO access sizes
From: Benjamin Herrenschmidt
It is common for PCI based UARTs to use larger than one byte access
sizes. This adds support for this and uses the information present
in SPCR accordingly.
Signed-off-by: Benjamin Herrenschmidt
---
grub-core/term/ns8250-spcr.c | 3 ++-
grub-core/term/ns8250.c | 45 +---
include/grub/serial.h| 9 ++--
3 files changed, 51 insertions(+), 6 deletions(-)
diff --git a/grub-core/term/ns8250-spcr.c b/grub-core/term/ns8250-spcr.c
index 0786aee1b..dd589af60 100644
--- a/grub-core/term/ns8250-spcr.c
+++ b/grub-core/term/ns8250-spcr.c
@@ -73,7 +73,8 @@ grub_ns8250_spcr_init (void)
switch (spcr->base_addr.space_id)
{
case GRUB_ACPI_GENADDR_MEM_SPACE:
-return grub_serial_ns8250_add_mmio(spcr->base_addr.addr, &config);
+return grub_serial_ns8250_add_mmio(spcr->base_addr.addr,
+ spcr->base_addr.access_size,
&config);
case GRUB_ACPI_GENADDR_IO_SPACE:
return grub_serial_ns8250_add_port(spcr->base_addr.addr, &config);
default:
diff --git a/grub-core/term/ns8250.c b/grub-core/term/ns8250.c
index 08dfb99da..76f68c037 100644
--- a/grub-core/term/ns8250.c
+++ b/grub-core/term/ns8250.c
@@ -48,7 +48,26 @@ ns8250_reg_read (struct grub_serial_port *port, grub_addr_t
reg)
{
asm volatile("" : : : "memory");
if (port->mmio)
-return *((volatile unsigned char *)(port->mmio_base + reg));
+{
+ /* Note: we assume MMIO UARTs are little endian. This is not true of all
+ * embedded platforms but will do for now
+ */
+ switch(port->access_size)
+{
+default:
+case 1:
+ return *((volatile unsigned char *)(port->mmio_base + reg));
+case 2:
+ return grub_le_to_cpu16(*((volatile unsigned short
*)(port->mmio_base + (reg << 1;
+case 3:
+ return grub_le_to_cpu32(*((volatile unsigned long *)(port->mmio_base
+ (reg << 2;
+case 4:
+ /* This will only work properly on 64-bit systems, thankfully it
+ * also probably doesn't exist
+ */
+ return grub_le_to_cpu64(*((volatile unsigned long long
*)(port->mmio_base + (reg << 3;
+}
+}
return grub_inb (port->port + reg);
}
@@ -57,7 +76,24 @@ ns8250_reg_write (struct grub_serial_port *port, unsigned
char value, grub_addr_
{
asm volatile("" : : : "memory");
if (port->mmio)
-*((volatile char *)(port->mmio_base + reg)) = value;
+{
+ switch(port->access_size)
+{
+default:
+case 1:
+ *((volatile unsigned char *)(port->mmio_base + reg)) = value;
+ break;
+case 2:
+ *((volatile unsigned short *)(port->mmio_base + (reg << 1))) =
grub_cpu_to_le16(value);
+ break;
+case 3:
+ *((volatile unsigned long *)(port->mmio_base + (reg << 2))) =
grub_cpu_to_le32(value);
+ break;
+case 4:
+ *((volatile unsigned long long *)(port->mmio_base + (reg << 3))) =
grub_cpu_to_le64(value);
+ break;
+}
+}
else
grub_outb (value, port->port + reg);
}
@@ -285,6 +321,7 @@ grub_ns8250_init (void)
grub_print_error ();
grub_serial_register (&com_ports[i]);
+ com_ports[i].access_size = 1;
}
}
@@ -338,6 +375,7 @@ grub_serial_ns8250_add_port (grub_port_t port, struct
grub_serial_config *config
p->driver = &grub_ns8250_driver;
p->mmio = 0;
p->port = port;
+ p->access_size = 1;
if (config)
grub_serial_port_configure (p, config);
else
@@ -348,7 +386,7 @@ grub_serial_ns8250_add_port (grub_port_t port, struct
grub_serial_config *config
}
char *
-grub_serial_ns8250_add_mmio(grub_addr_t addr, struct grub_serial_config
*config)
+grub_serial_ns8250_add_mmio(grub_addr_t addr, unsigned int acc_size, struct
grub_serial_config *config)
{
struct grub_serial_port *p;
unsigned i;
@@ -372,6 +410,7 @@ grub_serial_ns8250_add_mmio(grub_addr_t addr, struct
grub_serial_config *config)
p->driver = &grub_ns8250_driver;
p->mmio = 1;
p->mmio_base = addr;
+ p->access_size = acc_size;
if (config)
grub_serial_port_configure (p, config);
else
diff --git a/include/grub/serial.h b/include/grub/serial.h
index 646bdf1bb..8875eaf82 100644
--- a/include/grub/serial.h
+++ b/include/grub/serial.h
@@ -94,7 +94,12 @@ struct grub_serial_port
#if defined(__mips__) || defined (__i386__) || defined (__x86_64__)
grub_port_t port;
#endif
-grub_addr_t mmio_base;
+struct
+{
+ grub_addr_t mmio_base;
+ /* Access size uses ACPI definition */
+ unsigned int access_size;
+};
};
};
struct
@@ -187,7 +192,7 @@ grub_serial_config_defaults (struct grub_serial_port *port)
void grub_ns8250_init (void);
char * grub_ns8250_spcr_init (void);
char *grub_serial_ns8250_add_port (grub_port_t port, struct grub_serial_co
[PATCH 5/7] ns8250: Use ACPI SPCR table when available to configure serial
From: Benjamin Herrenschmidt
"serial auto" is now equivalent to just "serial" and will use the
SPCR to discover the port if present, otherwise defaults to "com0"
as before.
This allows to support MMIO ports specified by ACPI which is needed
on AWS EC2 "metal" instances, and will enable grub to pickup the
port configuration specified by ACPI in other cases.
Signed-off-by: Benjamin Herrenschmidt
---
docs/grub.texi | 10 -
grub-core/Makefile.core.def | 1 +
grub-core/term/ns8250-spcr.c | 82
grub-core/term/serial.c | 13 +-
include/grub/serial.h| 1 +
5 files changed, 105 insertions(+), 2 deletions(-)
create mode 100644 grub-core/term/ns8250-spcr.c
diff --git a/docs/grub.texi b/docs/grub.texi
index 50c811a88..bbebc2aef 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -2719,6 +2719,10 @@ you want to use COM2, you must specify @samp{--unit=1}
instead. This
command accepts many other options, so please refer to @ref{serial},
for more details.
+Without argument or with @samp{--port=auto}, GRUB will attempt to use
+ACPI when available to auto-detect the default serial port and its
+configuration.
+
The commands @command{terminal_input} (@pxref{terminal_input}) and
@command{terminal_output} (@pxref{terminal_output}) choose which type of
terminal you want to use. In the case above, the terminal will be a
@@ -2737,7 +2741,6 @@ implements few VT100 escape sequences. If you specify
this option then
GRUB provides you with an alternative menu interface, because the normal
menu requires several fancy features of your terminal.
-
@node Vendor power-on keys
@chapter Using GRUB with vendor power-on keys
@@ -4172,6 +4175,11 @@ be in the range 5-8 and stop bits must be 1 or 2.
Default is 8 data
bits and one stop bit. @var{parity} is one of @samp{no}, @samp{odd},
@samp{even} and defaults to @samp{no}.
+In passed no @var{unit} nor @var{port}, or if @var{port} is set to
+@samp{auto} then GRUB will attempt to use ACPI to automatically detect
+the system default serial port and its configuration. If this information
+is not available, it will default to @var{unit} 0.
+
The serial port is not used as a communication channel unless the
@command{terminal_input} or @command{terminal_output} command is used
(@pxref{terminal_input}, @pxref{terminal_output}).
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 95942fc8c..b656bdb44 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -2048,6 +2048,7 @@ module = {
name = serial;
common = term/serial.c;
x86 = term/ns8250.c;
+ x86 = term/ns8250-spcr.c;
ieee1275 = term/ieee1275/serial.c;
mips_arc = term/arc/serial.c;
efi = term/efi/serial.c;
diff --git a/grub-core/term/ns8250-spcr.c b/grub-core/term/ns8250-spcr.c
new file mode 100644
index 0..0786aee1b
--- /dev/null
+++ b/grub-core/term/ns8250-spcr.c
@@ -0,0 +1,82 @@
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2000,2001,2002,2003,2004,2005,2007,2008,2009,2010 Free
Software Foundation, Inc.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see
[PATCH 4/7] ns8250: Add configuration parameter when adding ports
From: Benjamin Herrenschmidt
This will allow ports to be added with a pre-set configuration
Signed-off-by: Benjamin Herrenschmidt
---
grub-core/term/ns8250.c | 25 +++--
grub-core/term/serial.c | 2 +-
include/grub/serial.h | 4 ++--
3 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/grub-core/term/ns8250.c b/grub-core/term/ns8250.c
index b640508d0..08dfb99da 100644
--- a/grub-core/term/ns8250.c
+++ b/grub-core/term/ns8250.c
@@ -303,7 +303,7 @@ grub_ns8250_hw_get_port (const unsigned int unit)
}
char *
-grub_serial_ns8250_add_port (grub_port_t port)
+grub_serial_ns8250_add_port (grub_port_t port, struct grub_serial_config
*config)
{
struct grub_serial_port *p;
unsigned i;
@@ -312,6 +312,9 @@ grub_serial_ns8250_add_port (grub_port_t port)
{
if (dead_ports & (1 << i))
return NULL;
+ /* give the opportunity for SPCR to configure a default com port */
+ if (config)
+ grub_serial_port_configure (&com_ports[i], config);
return com_names[i];
}
@@ -333,22 +336,29 @@ grub_serial_ns8250_add_port (grub_port_t port)
return NULL;
}
p->driver = &grub_ns8250_driver;
- grub_serial_config_defaults (p);
p->mmio = 0;
p->port = port;
+ if (config)
+grub_serial_port_configure (p, config);
+ else
+grub_serial_config_defaults (p);
grub_serial_register (p);
return p->name;
}
char *
-grub_serial_ns8250_add_mmio(grub_addr_t addr)
+grub_serial_ns8250_add_mmio(grub_addr_t addr, struct grub_serial_config
*config)
{
struct grub_serial_port *p;
unsigned i;
for (i = 0; i < GRUB_SERIAL_PORT_NUM; i++)
-if (com_ports[i].mmio && com_ports[i].mmio_base == addr)
- return com_names[i];
+if (com_ports[i].mmio && com_ports[i].mmio_base == addr)
+ {
+if (config)
+ grub_serial_port_configure (&com_ports[i], config);
+return com_names[i];
+ }
p = grub_malloc (sizeof (*p));
if (!p)
@@ -360,9 +370,12 @@ grub_serial_ns8250_add_mmio(grub_addr_t addr)
return NULL;
}
p->driver = &grub_ns8250_driver;
- grub_serial_config_defaults (p);
p->mmio = 1;
p->mmio_base = addr;
+ if (config)
+grub_serial_port_configure (p, config);
+ else
+grub_serial_config_defaults (p);
grub_serial_register (p);
return p->name;
diff --git a/grub-core/term/serial.c b/grub-core/term/serial.c
index c8f5f02db..a1707d2f7 100644
--- a/grub-core/term/serial.c
+++ b/grub-core/term/serial.c
@@ -156,7 +156,7 @@ grub_serial_find (const char *name)
&& grub_isxdigit (name [sizeof ("port") - 1]))
{
name = grub_serial_ns8250_add_port (grub_strtoul (&name[sizeof ("port")
- 1],
- 0, 16));
+ 0, 16), NULL);
if (!name)
return NULL;
diff --git a/include/grub/serial.h b/include/grub/serial.h
index ccf464d41..7efc956b0 100644
--- a/include/grub/serial.h
+++ b/include/grub/serial.h
@@ -185,8 +185,8 @@ grub_serial_config_defaults (struct grub_serial_port *port)
#if defined(__mips__) || defined (__i386__) || defined (__x86_64__)
void grub_ns8250_init (void);
-char *grub_serial_ns8250_add_port (grub_port_t port);
-char *grub_serial_ns8250_add_mmio(grub_addr_t addr);
+char *grub_serial_ns8250_add_port (grub_port_t port, struct grub_serial_config
*config);
+char *grub_serial_ns8250_add_mmio(grub_addr_t addr, struct grub_serial_config
*config);
#endif
#ifdef GRUB_MACHINE_IEEE1275
void grub_ofserial_init (void);
--
2.34.1
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 7/7] serial: Add ability to specify MMIO ports via 'serial'
From: Benjamin Herrenschmidt
This adds the ability to explicitely add an MMIO based serial port
via the 'serial' command. The syntax is:
serial --port=mmio,{.b,.w,.l,.q}
Signed-off-by: Benjamin Herrenschmidt
---
docs/grub.texi | 26 --
grub-core/term/serial.c | 31 ++-
2 files changed, 54 insertions(+), 3 deletions(-)
diff --git a/docs/grub.texi b/docs/grub.texi
index bbebc2aef..a25235636 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -4167,8 +4167,24 @@ Commands usable anywhere in the menu and in the
command-line.
@deffn Command serial [@option{--unit=unit}] [@option{--port=port}]
[@option{--speed=speed}] [@option{--word=word}] [@option{--parity=parity}]
[@option{--stop=stop}]
Initialize a serial device. @var{unit} is a number in the range 0-3
specifying which serial port to use; default is 0, which corresponds to
-the port often called COM1. @var{port} is the I/O port where the UART
-is to be found; if specified it takes precedence over @var{unit}.
+the port often called COM1.
+
+@var{port} is the I/O port where the UART is to be found or, if prefixed
+with @samp{mmio,}, the MMIO address of the UART. If specified it takes
+precedence over @var{unit}.
+
+Additionally, an MMIO address can be suffixed with:
+@itemize @bullet
+@item
+@samp{.b} for bytes access (default)
+@item
+@samp{.w} for 16-bit word access
+@item
+@samp{.l} for 32-bit long word access or
+@item
+@samp{.q} for 64-bit long long word access
+@end itemize
+
@var{speed} is the transmission speed; default is 9600. @var{word} and
@var{stop} are the number of data bits and stop bits. Data bits must
be in the range 5-8 and stop bits must be 1 or 2. Default is 8 data
@@ -4184,6 +4200,12 @@ The serial port is not used as a communication channel
unless the
@command{terminal_input} or @command{terminal_output} command is used
(@pxref{terminal_input}, @pxref{terminal_output}).
+Examples:
+@example
+serial --port=3f8 --speed=9600
+serial --port=mmio,fefb.l --speed=115200
+@end example
+
See also @ref{Serial terminal}.
@end deffn
diff --git a/grub-core/term/serial.c b/grub-core/term/serial.c
index f57fe45ef..96a5ad725 100644
--- a/grub-core/term/serial.c
+++ b/grub-core/term/serial.c
@@ -164,6 +164,35 @@ grub_serial_find (const char *name)
if (grub_strcmp (port->name, name) == 0)
break;
}
+ if (!port && grub_memcmp (name, "mmio,", sizeof ("mmio,") - 1) == 0
+ && grub_isxdigit (name [sizeof ("mmio,") - 1]))
+{
+ const char *p1, *p = &name[sizeof ("mmio,") - 1];
+ grub_addr_t addr = grub_strtoul (p, &p1, 16);
+ unsigned int acc_size = 1;
+ unsigned int nlen = p1 - p;
+ if (p1[0] == '.')
+switch(p1[1])
+ {
+ case 'w':
+acc_size = 2;
+break;
+ case 'l':
+acc_size = 3;
+break;
+ case 'q':
+acc_size = 4;
+break;
+ }
+ name = grub_serial_ns8250_add_mmio (addr, acc_size, NULL);
+ if (!name)
+ return NULL;
+
+ FOR_SERIAL_PORTS (port)
+if (grub_strncmp (port->name, name, nlen) == 0) {
+ break;
+ }
+}
if (!port && grub_strcmp (name, "auto") == 0)
{
/* Look for an SPCR if any. If not, default to com0 */
@@ -212,7 +241,7 @@ grub_cmd_serial (grub_extcmd_context_t ctxt, int argc, char
**args)
if (state[OPTION_PORT].set)
{
- if (grub_memcmp (state[OPTION_PORT].arg, "mmio", 4) == 0)
+ if (grub_memcmp (state[OPTION_PORT].arg, "mmio,", 5) == 0)
grub_snprintf(pname, sizeof (pname), "%s", state[1].arg);
else
grub_snprintf (pname, sizeof (pname), "port%lx",
--
2.34.1
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
