Gianni Tedesco
2010-Jul-22 17:42 UTC
[Xen-devel] [PATCH]: Fix rombios to correctly report size of disks >16GB
Cosmetic patch for rombios to display the correct sizes of disks larger
than 16GB - a problem caused by needlessly casting sizeinmb variable
down to 16bits.
Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
diff -r 01917c0da12a tools/firmware/rombios/rombios.c
--- a/tools/firmware/rombios/rombios.c Thu Jul 22 13:49:40 2010 +0100
+++ b/tools/firmware/rombios/rombios.c Thu Jul 22 18:40:26 2010 +0100
@@ -2856,9 +2856,9 @@ void ata_detect( )
printf("ata%d %s: ",channel,slave?"
slave":"master");
i=0; while(c=read_byte(get_SS(),model+i++)) printf("%c",c);
if (sizeinmb < (1UL<<16))
- printf(" ATA-%d Hard-Disk (%4u MBytes)\n", version,
(Bit16u)sizeinmb);
+ printf(" ATA-%d Hard-Disk (%4u MBytes)\n", version,
sizeinmb);
else
- printf(" ATA-%d Hard-Disk (%4u GBytes)\n", version,
(Bit16u)(sizeinmb>>10));
+ printf(" ATA-%d Hard-Disk (%4u GBytes)\n", version,
(sizeinmb>>10));
break;
case ATA_TYPE_ATAPI:
printf("ata%d %s: ",channel,slave?"
slave":"master");
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Keir Fraser
2010-Jul-22 18:07 UTC
Re: [Xen-devel] [PATCH]: Fix rombios to correctly report size of disks >16GB
On 22/07/2010 18:42, "Gianni Tedesco" <gianni.tedesco@citrix.com> wrote:> Cosmetic patch for rombios to display the correct sizes of disks larger > than 16GB - a problem caused by needlessly casting sizeinmb variable > down to 16bits.For say a 17GB drive sizeinmb=17000 which can be represented with fewer than 16 bits. I don''t see how the code as it is will ever print the wrong thing, except for drives bigger than 64TB! Also I''m not sure but I think the integer size here may be 16 bits, and the %u format specifier would only print 16-bit values in that case anyway. -- Keir> Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com> > > diff -r 01917c0da12a tools/firmware/rombios/rombios.c > --- a/tools/firmware/rombios/rombios.c Thu Jul 22 13:49:40 2010 +0100 > +++ b/tools/firmware/rombios/rombios.c Thu Jul 22 18:40:26 2010 +0100 > @@ -2856,9 +2856,9 @@ void ata_detect( ) > printf("ata%d %s: ",channel,slave?" slave":"master"); > i=0; while(c=read_byte(get_SS(),model+i++)) printf("%c",c); > if (sizeinmb < (1UL<<16)) > - printf(" ATA-%d Hard-Disk (%4u MBytes)\n", version, > (Bit16u)sizeinmb); > + printf(" ATA-%d Hard-Disk (%4u MBytes)\n", version, sizeinmb); > else > - printf(" ATA-%d Hard-Disk (%4u GBytes)\n", version, > (Bit16u)(sizeinmb>>10)); > + printf(" ATA-%d Hard-Disk (%4u GBytes)\n", version, > (sizeinmb>>10)); > break; > case ATA_TYPE_ATAPI: > printf("ata%d %s: ",channel,slave?" slave":"master"); > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-23 08:07 UTC
Re: [Xen-devel] [PATCH]: Fix rombios to correctly report size of disks >16GB
On Thu, 2010-07-22 at 19:07 +0100, Keir Fraser wrote:> On 22/07/2010 18:42, "Gianni Tedesco" <gianni.tedesco@citrix.com> wrote: > > > Cosmetic patch for rombios to display the correct sizes of disks larger > > than 16GB - a problem caused by needlessly casting sizeinmb variable > > down to 16bits. > > For say a 17GB drive sizeinmb=17000 which can be represented with fewer than > 16 bits. I don''t see how the code as it is will ever print the wrong thing, > except for drives bigger than 64TB!Actually you are right then wrong, the patch is mis-titled, it''s 64GB. (in fact 65,535 MB).> Also I''m not sure but I think the > integer size here may be 16 bits, and the %u format specifier would only > print 16-bit values in that case anyway.I tested it and it works as advertised (modulo 16 -> 64).... Gianni _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-23 08:14 UTC
Re: [Xen-devel] [PATCH]: Fix rombios to correctly report size of disks >16GB
On Fri, 2010-07-23 at 09:07 +0100, Gianni Tedesco wrote:> On Thu, 2010-07-22 at 19:07 +0100, Keir Fraser wrote: > > On 22/07/2010 18:42, "Gianni Tedesco" <gianni.tedesco@citrix.com> wrote: > > > > > Cosmetic patch for rombios to display the correct sizes of disks larger > > > than 16GB - a problem caused by needlessly casting sizeinmb variable > > > down to 16bits. > > > > For say a 17GB drive sizeinmb=17000 which can be represented with fewer than > > 16 bits. I don''t see how the code as it is will ever print the wrong thing, > > except for drives bigger than 64TB! > > Actually you are right then wrong, the patch is mis-titled, it''s 64GB. > (in fact 65,535 MB). > > > Also I''m not sure but I think the > > integer size here may be 16 bits, and the %u format specifier would only > > print 16-bit values in that case anyway. > > I tested it and it works as advertised (modulo 16 -> 64)....BTW. the reason for that is printf() and integer definition doesn''t change regardless of whether gcc is compiling for 16 or 32 bit mode but just changes the logic wrt operand-size insn prefix Gianni _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Jul-23 08:21 UTC
Re: [Xen-devel] [PATCH]: Fix rombios to correctly report size of disks >16GB
On 23/07/2010 09:07, "Gianni Tedesco (3P)" <gianni.tedesco@citrix.com> wrote:>> For say a 17GB drive sizeinmb=17000 which can be represented with fewer than >> 16 bits. I don''t see how the code as it is will ever print the wrong thing, >> except for drives bigger than 64TB! > > Actually you are right then wrong, the patch is mis-titled, it''s 64GB. > (in fact 65,535 MB). > >> Also I''m not sure but I think the >> integer size here may be 16 bits, and the %u format specifier would only >> print 16-bit values in that case anyway. > > I tested it and it works as advertised (modulo 16 -> 64)....64GB == 65536MB > 1UL<<16, hence we should print the size in GB, and shift right 10 before casting. Hence should still print correctly. Is this another example where the bcc compiler is broken? -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Jul-23 08:30 UTC
Re: [Xen-devel] [PATCH]: Fix rombios to correctly report size of disks >16GB
On 23/07/2010 09:14, "Gianni Tedesco (3P)" <gianni.tedesco@citrix.com> wrote:>>> Also I''m not sure but I think the >>> integer size here may be 16 bits, and the %u format specifier would only >>> print 16-bit values in that case anyway. >> >> I tested it and it works as advertised (modulo 16 -> 64).... > > BTW. the reason for that is printf() and integer definition doesn''t > change regardless of whether gcc is compiling for 16 or 32 bit mode but > just changes the logic wrt operand-size insn prefixThis isn''t compiled with gcc, but with bcc from the dev86 toolchain. Hacking sizeinmb to be 67UL<<10, I find that 67GB is printed which is correct. That''s with the latest dev86 (which is still very old!) v0.16.17. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Jul-23 08:32 UTC
Re: [Xen-devel] [PATCH]: Fix rombios to correctly report size of disks >16GB
On Fri, 2010-07-23 at 09:21 +0100, Keir Fraser wrote:> On 23/07/2010 09:07, "Gianni Tedesco (3P)" <gianni.tedesco@citrix.com> > wrote: > > >> For say a 17GB drive sizeinmb=17000 which can be represented with fewer than > >> 16 bits. I don''t see how the code as it is will ever print the wrong thing, > >> except for drives bigger than 64TB! > > > > Actually you are right then wrong, the patch is mis-titled, it''s 64GB. > > (in fact 65,535 MB). > > > >> Also I''m not sure but I think the > >> integer size here may be 16 bits, and the %u format specifier would only > >> print 16-bit values in that case anyway. > > > > I tested it and it works as advertised (modulo 16 -> 64).... > > 64GB == 65536MB > 1UL<<16, hence we should print the size in GB, and shift > right 10 before casting. Hence should still print correctly. Is this another > example where the bcc compiler is broken?Argh, ignore me, you are right - I was mindlessly sending off my patch-queue against xenserver in which there are no brackets on the bit-shift! :) Sorry for the noise _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel