Re: [LinuxBIOS] r2890 - in trunk/LinuxBIOSv2/src: arch/i386/boot boot cpu/x86/mtrr cpu/x86/tsc devices

[Uwe Hermann]

On Wed, Oct 24, 2007 at 12:17:45AM +0200, [e-mail] wrote:
> Modified: trunk/LinuxBIOSv2/src/arch/i386/boot/linuxbios_table.c
>
> --- trunk/LinuxBIOSv2/src/arch/i386/boot/linuxbios_table.c	2007-10-23 19:23:52 UTC (rev 2889)
> +++ trunk/LinuxBIOSv2/src/arch/i386/boot/linuxbios_table.c	2007-10-23 22:17:45 UTC (rev 2890)
> @@ -120,7 +120,7 @@
>  {
>    static const struct {
>    	uint32_t tag;
> const uint8_t *string;
> const char *string;

Yep.
> Modified: trunk/LinuxBIOSv2/src/boot/elfboot.c
>
> --- trunk/LinuxBIOSv2/src/boot/elfboot.c	2007-10-23 19:23:52 UTC (rev 2889)
> +++ trunk/LinuxBIOSv2/src/boot/elfboot.c	2007-10-23 22:17:45 UTC (rev 2890)
> @@ -144,7 +144,7 @@
>  {
>    struct verify_callback *cb_chain;
>    unsigned char *note, *end;
> char *program, *version;
> unsigned char *program, *version;

This doesn't look like the correct fix. Strings should always be 'char *'
or 'const char *' IMO, so the code where program/version is used should
be fixed instead.
> Modified: trunk/LinuxBIOSv2/src/devices/device_util.c
>
> --- trunk/LinuxBIOSv2/src/devices/device_util.c	2007-10-23 19:23:52 UTC (rev 2889)
> +++ trunk/LinuxBIOSv2/src/devices/device_util.c	2007-10-23 22:17:45 UTC (rev 2890)
> @@ -454,7 +454,7 @@
>  void report_resource_stored(device_t dev, struct resource *resource, const char *comment)
>  {
>    if (resource->flags & IORESOURCE_STORED) {
> unsigned char buf[10];
> char buf[10];

If buf contains a string, yes. If it should contain 8bit data, it should
be uint8_t or u8.
> unsigned long long base, end;
> base = resource->base;
> end = resource_end(resource);
>
> Modified: trunk/LinuxBIOSv2/src/devices/hypertransport.c
>
> --- trunk/LinuxBIOSv2/src/devices/hypertransport.c	2007-10-23 19:23:52 UTC (rev 2889)
> +++ trunk/LinuxBIOSv2/src/devices/hypertransport.c	2007-10-23 22:17:45 UTC (rev 2890)
> @@ -37,6 +37,9 @@
>  #include <part/hard_reset.h>
>  #include <part/fallback_boot.h>
>
> +/* The hypertransport link is already optimized in pre-ram code
> so don't do it again
> */
>
> #define OPT_HT_LINK 0
>
> #if OPT_HT_LINK == 1
> @@ -123,15 +126,17 @@
>
> static int ht_setup_link(struct ht_link *prev, device_t dev, unsigned pos)
>  {
> +#if OPT_HT_LINK == 1
>    static const uint8_t link_width_to_pow2[]= { 3, 4, 0, 5, 1, 2, 0, 0 };
>    static const uint8_t pow2_to_link_width[] = { 0x7, 4, 5, 0, 1, 3 };
> struct ht_link cur[1];
> unsigned present_width_cap,    upstream_width_cap;
> unsigned present_freq_cap,     upstream_freq_cap;
> unsigned ln_present_width_in,  ln_upstream_width_in; 
> unsigned ln_present_width_out, ln_upstream_width_out;
> unsigned freq, old_freq;
> unsigned present_width, upstream_width, old_width;
>
> +#endif
> struct ht_link cur[1];

This change is not trivial, as it might very well have effects on the
run-time behaviour of LinuxBIOS / the hardware.

Can we say for sure that there's no reason to (re-)initialize here?
Is this tested on hardware?

Uwe.