Re: [Xen-devel] [PATCH] Increment buffer used to read first boot sector in order to accomodate space for 4k sector

[Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>]

On Fri, 2012-08-03 at 16:22 +0100, Jan Beulich wrote:
> >>> On 03.08.12 at 16:50, Frediano Ziglio <frediano.ziglio@xxxxxxxxxx> wrote:
> > If a 4k disk is used for first BIOS disk loader corrupt itself.
> 
> If such is really permitted by the specification (which I doubt it
> is for the standard, old-style INT13 functions - it's a different
> story for functions 42 and 43, where the caller can be expected
> to call function 48 first).
> 

I don't know. They always speaks about sector size and in int13/5 for
floppy you can specify different sector sizes (up to 1024).

> > This patch increase sector buffer in order to avoid this overflow
> 
> And if we indeed need to adjust for this, then let's fix this properly:
> Don't just increase the buffer size, but also check that the sector
> size reported actually fits. That may require calling Fn48 first,
> before doing the actual read.
> 

Or read to a location of memory we are sure there is enough space
(something like 2000:0000).

> > --- a/xen/arch/x86/boot/edd.S
> > +++ b/xen/arch/x86/boot/edd.S
> > @@ -154,4 +154,4 @@ boot_mbr_signature_nr:
> >  boot_mbr_signature:
> >          .fill   EDD_MBR_SIG_MAX*8,1,0
> >  boot_edd_info:
> > -        .fill   512,1,0                         # big enough for a disc 
> > sector
> > +        .fill   4096,1,0                         # big enough for a disc 
> > sector
> 
> Also I wonder whether it wouldn't be more smart to re-use the
> wakeup stack (which is already 4k in size), and shrink this buffer
> to the maximum size ever used without reading sectors into it
> (EDD_INFO_MAX*(EDDEXTSIZE+EDDPARMSIZE)).
> 

Yes, reusing this buffer could be useful. It could be also useful to put
it at the end of the trampoline code in order to try avoiding future
problems if sector size grow.

> > --- a/xen/arch/x86/boot/trampoline.S
> > +++ b/xen/arch/x86/boot/trampoline.S
> > @@ -224,6 +224,6 @@ skip_realmode:
> >  rm_idt: .word   256*4-1, 0, 0
> >  
> >  #include "mem.S"
> > -#include "edd.S"
> >  #include "video.S"
> >  #include "wakeup.S"
> > +#include "edd.S"
> 
> Finally, you should also explain why this change is needed.
> 

This is to move the buffer at the end and avoid overflowing to other
code.

> Jan
> 

Frediano

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel