[PATCH] fix two issues with virtio-blk

[Gleb Natapov]

1. Check if blk_size is valid in virtio_blk config.
2. Disable interrupt otherwise interrupt may stuck
   with some guests.

Signed-off-by: Gleb Natapov 
diff --git a/src/virtio-blk.c b/src/virtio-blk.c
index 6c3f8a5..96334b1 100644
--- a/src/virtio-blk.c
+++ b/src/virtio-blk.c
@@ -138,7 +138,9 @@ virtio_blk_setup(void)
         struct virtio_blk_config cfg;
         vp_get(ioaddr, 0, &cfg, sizeof(cfg));

-        vdrive_g->drive.blksize = cfg.blk_size;
+        u32 f = vp_get_features(ioaddr);
+        vdrive_g->drive.blksize = (f & (1 << VIRTIO_BLK_F_BLK_SIZE)) ?
+            cfg.blk_size : DISK_SECTOR_SIZE;
         vdrive_g->drive.sectors = cfg.capacity;
         dprintf(3, "virtio-blk %x:%x blksize=%d sectors=%u\n",
                 pci_bdf_to_bus (bdf), pci_bdf_to_dev(bdf),
diff --git a/src/virtio-blk.h b/src/virtio-blk.h
index 8095d5b..7243704 100644
--- a/src/virtio-blk.h
+++ b/src/virtio-blk.h
@@ -16,6 +16,8 @@ struct virtio_blk_config
     u32 opt_io_size;
 } __attribute__((packed));

+#define VIRTIO_BLK_F_BLK_SIZE 6
+
 /* These two define direction. */
 #define VIRTIO_BLK_T_IN		0
 #define VIRTIO_BLK_T_OUT	1
diff --git a/src/virtio-ring.h b/src/virtio-ring.h
index 95ae85b..f2f2460 100644
--- a/src/virtio-ring.h
+++ b/src/virtio-ring.h
@@ -105,6 +105,8 @@ static inline void vring_init(struct vring *vr,
    vr->desc = phys_to_virt(pa);

vr->avail = (struct vring_avail *)&vr->desc[num];
+   /* disable interrupts */
+   vr->avail->flags |= VRING_AVAIL_F_NO_INTERRUPT;

/* physical address of used must be page aligned */

[Stefan Hajnoczi]

On Tue, May 11, 2010 at 1:21 PM, Gleb Natapov  wrote:
> Disable interrupt otherwise interrupt may stuck
>
> with some guests.

[...]
> diff --git a/src/virtio-ring.h b/src/virtio-ring.h
> index 95ae85b..f2f2460 100644
> --- a/src/virtio-ring.h
> +++ b/src/virtio-ring.h
> @@ -105,6 +105,8 @@ static inline void vring_init(struct vring *vr,
>    vr->desc = phys_to_virt(pa);
>
> vr->avail = (struct vring_avail *)&vr->desc[num];
> /* disable interrupts */
> vr->avail->flags |= VRING_AVAIL_F_NO_INTERRUPT;

I don't understand this.  Why would the interrupt get stuck?

The virtio-net device closely manages interrupts because it uses NAPI.
 The virtio-blk device assumes it will receive interrupts and doesn't
enable_cb/disable_cb.  I might be confused but have you tested this
change with virtio-blk?

Stefan

[Gleb Natapov]

On Wed, May 12, 2010 at 04:42:30PM +0100, Stefan Hajnoczi wrote:
> On Tue, May 11, 2010 at 1:21 PM, Gleb Natapov  wrote:
> > Disable interrupt otherwise interrupt may stuck
> >
> > with some guests.
>
> [...]
> > diff --git a/src/virtio-ring.h b/src/virtio-ring.h
> > index 95ae85b..f2f2460 100644
> > --- a/src/virtio-ring.h
> > +++ b/src/virtio-ring.h
> > @@ -105,6 +105,8 @@ static inline void vring_init(struct vring *vr,
> >    vr->desc = phys_to_virt(pa);
> >
> > vr->avail = (struct vring_avail *)&vr->desc[num];
> > /* disable interrupts */
> > vr->avail->flags |= VRING_AVAIL_F_NO_INTERRUPT;
>
> I don't understand this.  Why would the interrupt get stuck?

Because virtio-blk will send interrupt on read completion, but nobody
ever clears it and if OS will unmask the interrupt before virtio-blk
initialization nobody will know how to ack it.
> The virtio-net device closely manages interrupts because it uses NAPI.
>  The virtio-blk device assumes it will receive interrupts and doesn't
> enable_cb/disable_cb.  I might be confused but have you tested this
> change with virtio-blk?

This is per virt queue flag. virtio-blk creates another virt queue
without the flag. It works for me with virtio-blk.

[Stefan Hajnoczi]

Figured out the source of my confusion: I was looking at linux virtio,
not seabios!  I missed the [SeaBIOS] tag and thought this was a Linux
patch. :P

Stefan

[Kevin O'Connor]

On Tue, May 11, 2010 at 03:21:09PM +0300, Gleb Natapov wrote:
> Check if blk_size is valid in virtio_blk config.
> Disable interrupt otherwise interrupt may stuck
> with some guests.
>
> Signed-off-by: Gleb Natapov 
> diff --git a/src/virtio-blk.c b/src/virtio-blk.c
> index 6c3f8a5..96334b1 100644
> --- a/src/virtio-blk.c
> +++ b/src/virtio-blk.c
> @@ -138,7 +138,9 @@ virtio_blk_setup(void)
>          struct virtio_blk_config cfg;
>          vp_get(ioaddr, 0, &cfg, sizeof(cfg));
>    vdrive_g->drive.blksize = cfg.blk_size;
>    u32 f = vp_get_features(ioaddr);
>    vdrive_g->drive.blksize = (f & (1 << VIRTIO_BLK_F_BLK_SIZE)) ?
>        cfg.blk_size : DISK_SECTOR_SIZE;

The blksize needs to be 512, because the BIOS interface that virtio
binds to requires 512 byte sectors.  Shouldn't this look something
like:

if (cfg.blk_size != DISK_SECTOR_SIZE)
              goto fail;
          vdrive_g->drive.blksize = DISK_SECTOR_SIZE;

-Kevin

[Gleb Natapov]

On Wed, May 12, 2010 at 07:25:55PM -0400, Kevin O'Connor wrote:
> On Tue, May 11, 2010 at 03:21:09PM +0300, Gleb Natapov wrote:
> > Check if blk_size is valid in virtio_blk config.
> > Disable interrupt otherwise interrupt may stuck
> > with some guests.
> >
> > Signed-off-by: Gleb Natapov 
> > diff --git a/src/virtio-blk.c b/src/virtio-blk.c
> > index 6c3f8a5..96334b1 100644
> > --- a/src/virtio-blk.c
> > +++ b/src/virtio-blk.c
> > @@ -138,7 +138,9 @@ virtio_blk_setup(void)
> >          struct virtio_blk_config cfg;
> >          vp_get(ioaddr, 0, &cfg, sizeof(cfg));
> >    vdrive_g->drive.blksize = cfg.blk_size;
> >    u32 f = vp_get_features(ioaddr);
> >    vdrive_g->drive.blksize = (f & (1 << VIRTIO_BLK_F_BLK_SIZE)) ?
> >        cfg.blk_size : DISK_SECTOR_SIZE;
>
> The blksize needs to be 512, because the BIOS interface that virtio
> binds to requires 512 byte sectors.  Shouldn't this look something
> like:
>       if (cfg.blk_size != DISK_SECTOR_SIZE)
>           goto fail;
>       vdrive_g->drive.blksize = DISK_SECTOR_SIZE;

Hmm, I guess you are right. Boot indeed fails if I configure logical
block size bigger then 512 bytes. But shouldn't BIOS emulate 512 byte
access on top of bigger block size? Future disks will use much large
logical block sizes.

[Kevin O'Connor]

On Thu, May 13, 2010 at 02:00:24PM +0300, Gleb Natapov wrote:
> On Wed, May 12, 2010 at 07:25:55PM -0400, Kevin O'Connor wrote:
> > The blksize needs to be 512, because the BIOS interface that virtio
> > binds to requires 512 byte sectors.  Shouldn't this look something
> > like:

[...]
> Hmm, I guess you are right. Boot indeed fails if I configure logical
> block size bigger then 512 bytes. But shouldn't BIOS emulate 512 byte
> access on top of bigger block size? Future disks will use much large
> logical block sizes.

SeaBIOS only supports 512 byte sectors for floppies/harddrives and
only supports 2048 byte sectors for cdroms.  I suppose one could
implement an emulation layer (similar to the way the "El Torito" spec
can make a cdrom look like a harddrive), but so far nothing has
required that.

I suspect future drives will boot in a legacy mode that uses a 512
byte block interface - exactly because of these types of issues.

-Kevin

[Carl-Daniel Hailfinger]

On 14.05.2010 05:07, Kevin O'Connor wrote:
> SeaBIOS only supports 512 byte sectors for floppies/harddrives and
> only supports 2048 byte sectors for cdroms.  I suppose one could
> implement an emulation layer (similar to the way the "El Torito" spec
> can make a cdrom look like a harddrive), but so far nothing has
> required that.
>
> I suspect future drives will boot in a legacy mode that uses a 512
> byte block interface - exactly because of these types of issues.

The following links may provide some helpful insight:
http://lwn.net/Articles/322777/
https://ata.wiki.kernel.org/index.php/ATA_4_KiB_sector_issues

Regards,
Carl-Daniel

[Sebastian Herbszt]

Gleb Natapov wrote:
> On Wed, May 12, 2010 at 07:25:55PM -0400, Kevin O'Connor wrote:
> > The blksize needs to be 512, because the BIOS interface that virtio
> > binds to requires 512 byte sectors.  Shouldn't this look something
> > like:
> >       if (cfg.blk_size != DISK_SECTOR_SIZE)
> >           goto fail;
> >       vdrive_g->drive.blksize = DISK_SECTOR_SIZE;
>
> Hmm, I guess you are right. Boot indeed fails if I configure logical
> block size bigger then 512 bytes. But shouldn't BIOS emulate 512 byte
> access on top of bigger block size? Future disks will use much large
> logical block sizes.

What the BIOS should do or not doesn't seem to be defined anywhere (yet).
Current drives with physical sector size > 512 bytes report a logical sector
size of 512 bytes and do the emulation on their own. The BIOS doesn't need
to be modified so far to support those drives. If drives with logical sector
size > 512 bytes appear this situation might change. Doing a read emulation
in BIOS shouldn't be too bad, but a write emulation will need a "physical
sector size" RMW buffer (no clue where the BIOS could steal those 4KB from).
Maybe we are all using EFI without legacy BIOS interfaces when those drives
appear anyway.

Sebastian

[Kevin O'Connor]

On Sat, May 15, 2010 at 05:17:31PM +0200, Sebastian Herbszt wrote:
> Doing a read emulation
> in BIOS shouldn't be too bad, but a write emulation will need a "physical
> sector size" RMW buffer (no clue where the BIOS could steal those 4KB from).

SeaBIOS has a "malloc_low" function which obtains permanent memory in
the first 1Meg.  Currently, it grabs memory at the end of 640K (it
moves the ebda down if needed).  It's probably possible to use memory
in the c/d/e/f segments - though that's tricky because parts of that
area are likely to be read-only.

Right now SeaBIOS allocates a 2K buffer to support cdrom emulation
(this is different from what bochs bios does).  So, going to a 4K
buffer is probably reasonable.

-Kevin

[Sebastian Herbszt]

Kevin O'Connor wrote:
> On Sat, May 15, 2010 at 05:17:31PM +0200, Sebastian Herbszt wrote:
> > Doing a read emulation
> > in BIOS shouldn't be too bad, but a write emulation will need a "physical
> > sector size" RMW buffer (no clue where the BIOS could steal those 4KB from).
>
> SeaBIOS has a "malloc_low" function which obtains permanent memory in
> the first 1Meg.  Currently, it grabs memory at the end of 640K (it
> moves the ebda down if needed).  It's probably possible to use memory
> in the c/d/e/f segments - though that's tricky because parts of that
> area are likely to be read-only.
>
> Right now SeaBIOS allocates a 2K buffer to support cdrom emulation
> (this is different from what bochs bios does).  So, going to a 4K
> buffer is probably reasonable.
>
> -Kevin

This explains why i see 2 KB less free memory with SeaBIOS.
With current SeaBIOS i get 511 KB free memory running MS-DOS 6.22
with CDROM support. Sure you can grab another 4 KB for a RMW buffer,
but sooner or later there won't be enough memory free to play your favorite
game from the 90s. And i didn't even load an option ROM which also wants
1-2 KB EBDA or some useful TSR program.

Why is a 4 KB physical sector size needed in a VM anyway?

Sebastian

[Kevin O'Connor]

On Sat, May 15, 2010 at 10:08:54PM +0200, Sebastian Herbszt wrote:
> Kevin O'Connor wrote:
> > On Sat, May 15, 2010 at 05:17:31PM +0200, Sebastian Herbszt wrote:
> > > Doing a read emulation
> > > in BIOS shouldn't be too bad, but a write emulation will need a "physical
> > > sector size" RMW buffer (no clue where the BIOS could steal those 4KB from).
> >
> > SeaBIOS has a "malloc_low" function which obtains permanent memory in
> > the first 1Meg.  Currently, it grabs memory at the end of 640K (it
> > moves the ebda down if needed).  It's probably possible to use memory
> > in the c/d/e/f segments - though that's tricky because parts of that
> > area are likely to be read-only.
> >
> > Right now SeaBIOS allocates a 2K buffer to support cdrom emulation
> > (this is different from what bochs bios does).  So, going to a 4K
> > buffer is probably reasonable.
>
> This explains why i see 2 KB less free memory with SeaBIOS.
> With current SeaBIOS i get 511 KB free memory running MS-DOS 6.22
> with CDROM support. Sure you can grab another 4 KB for a RMW buffer,

It's only 2KB more - the buffer could be shared.
> but sooner or later there won't be enough memory free to play your favorite
> game from the 90s. And i didn't even load an option ROM which also wants
> 1-2 KB EBDA or some useful TSR program.
>
> Why is a 4 KB physical sector size needed in a VM anyway?

It isn't - I was just describing where one could get the memory.  I'm
not planning on implementing this.

There may come a point where drives require a bigger sector size.
(SeaBIOS can also run on real hardware.)  In that case, it may be
necessary to add support - after all, one can't run old games if their
computer doesn't boot.  However, I think we can cross that bridge when
we get to it.

-Kevin

[Sebastian Herbszt]

Kevin O'Connor wrote:
> On Sat, May 15, 2010 at 10:08:54PM +0200, Sebastian Herbszt wrote:
> > Kevin O'Connor wrote:
> > > On Sat, May 15, 2010 at 05:17:31PM +0200, Sebastian Herbszt wrote:
> > > > Doing a read emulation
> > > > in BIOS shouldn't be too bad, but a write emulation will need a "physical
> > > > sector size" RMW buffer (no clue where the BIOS could steal those 4KB from).
> > >
> > > SeaBIOS has a "malloc_low" function which obtains permanent memory in
> > > the first 1Meg.  Currently, it grabs memory at the end of 640K (it
> > > moves the ebda down if needed).  It's probably possible to use memory
> > > in the c/d/e/f segments - though that's tricky because parts of that
> > > area are likely to be read-only.
> > >
> > > Right now SeaBIOS allocates a 2K buffer to support cdrom emulation
> > > (this is different from what bochs bios does).  So, going to a 4K
> > > buffer is probably reasonable.
> >
> > This explains why i see 2 KB less free memory with SeaBIOS.
> > With current SeaBIOS i get 511 KB free memory running MS-DOS 6.22
> > with CDROM support. Sure you can grab another 4 KB for a RMW buffer,
>
> It's only 2KB more - the buffer could be shared.

Even if you want to be able to copy from CDROM emulation to a disk with 4 KB
physical sectors? I assume you need 2 KB for CDROM emulation and 4 KB for RMW.
> > but sooner or later there won't be enough memory free to play your favorite
> > game from the 90s. And i didn't even load an option ROM which also wants
> > 1-2 KB EBDA or some useful TSR program.
> >
> > Why is a 4 KB physical sector size needed in a VM anyway?
>
> It isn't - I was just describing where one could get the memory.  I'm
> not planning on implementing this.
>
> There may come a point where drives require a bigger sector size.
> (SeaBIOS can also run on real hardware.)  In that case, it may be
> necessary to add support - after all, one can't run old games if their
> computer doesn't boot.  However, I think we can cross that bridge when
> we get to it.
>
> -Kevin

Even if drives use a bigger physical sector size they can still use a logical sector
size of 512 bytes. This is what those drives with the Advanced Format Technology
do. The drive does the emulation/RMW and the BIOS gets its 512 byte logical sectors
and nothing changes for the software.

I assume the drive manufacturers and BIOS vendors will fix this if the emulation is
a real problem. Maybe a new ATA command can be introduced to ask the drive to
use a larger logical sector size. This way all drives could start up in compatibility
mode with emulation enabled. All legacy applications (BIOS, option ROMS, boot
loaders) will still work and if some advanced OS can use larger logical sectors it
could ask the drive to deliver those.

Sebastian

[Kevin O'Connor]

On Sun, May 16, 2010 at 02:59:19PM +0200, Sebastian Herbszt wrote:
> Kevin O'Connor wrote:
> > On Sat, May 15, 2010 at 10:08:54PM +0200, Sebastian Herbszt wrote:
> > > with CDROM support. Sure you can grab another 4 KB for a RMW buffer,
> >
> > It's only 2KB more - the buffer could be shared.
>
> Even if you want to be able to copy from CDROM emulation to a disk with 4 KB
> physical sectors? I assume you need 2 KB for CDROM emulation and 4 KB for RMW.

There is no "copy" BIOS command.  To copy data, the caller must read a
sector into memory and then write the sector from its memory to the
new disk.  Both read and write BIOS operations can use the same 4KB
buffer.
> > > Why is a 4 KB physical sector size needed in a VM anyway?
> >
> > It isn't - I was just describing where one could get the memory.  I'm
> > not planning on implementing this.

[...]
> Even if drives use a bigger physical sector size they can still use
> a logical sector size of 512 bytes. This is what those drives with
> the Advanced Format Technology

I think we are both in agreement.

-Kevin

[Sebastian Herbszt]

Kevin O'Connor wrote:
> On Sun, May 16, 2010 at 02:59:19PM +0200, Sebastian Herbszt wrote:
> > Kevin O'Connor wrote:
> > > On Sat, May 15, 2010 at 10:08:54PM +0200, Sebastian Herbszt wrote:
> > > > with CDROM support. Sure you can grab another 4 KB for a RMW buffer,
> > >
> > > It's only 2KB more - the buffer could be shared.
> >
> > Even if you want to be able to copy from CDROM emulation to a disk with 4 KB
> > physical sectors? I assume you need 2 KB for CDROM emulation and 4 KB for RMW.
>
> There is no "copy" BIOS command.  To copy data, the caller must read a
> sector into memory and then write the sector from its memory to the
> new disk.  Both read and write BIOS operations can use the same 4KB
> buffer.

Oops - you are right. My bad.

Sebastian

[Gleb Natapov]

On Sat, May 15, 2010 at 10:08:54PM +0200, Sebastian Herbszt wrote:
> Kevin O'Connor wrote:
> > On Sat, May 15, 2010 at 05:17:31PM +0200, Sebastian Herbszt wrote:
> > > Doing a read emulation
> > > in BIOS shouldn't be too bad, but a write emulation will need a "physical
> > > sector size" RMW buffer (no clue where the BIOS could steal those 4KB from).
> >
> > SeaBIOS has a "malloc_low" function which obtains permanent memory in
> > the first 1Meg.  Currently, it grabs memory at the end of 640K (it
> > moves the ebda down if needed).  It's probably possible to use memory
> > in the c/d/e/f segments - though that's tricky because parts of that
> > area are likely to be read-only.
> >
> > Right now SeaBIOS allocates a 2K buffer to support cdrom emulation
> > (this is different from what bochs bios does).  So, going to a 4K
> > buffer is probably reasonable.
> >
> > -Kevin
>
> This explains why i see 2 KB less free memory with SeaBIOS.
> With current SeaBIOS i get 511 KB free memory running MS-DOS 6.22
> with CDROM support. Sure you can grab another 4 KB for a RMW buffer,
> but sooner or later there won't be enough memory free to play your favorite
> game from the 90s. And i didn't even load an option ROM which also wants
> 1-2 KB EBDA or some useful TSR program.
>
> Why is a 4 KB physical sector size needed in a VM anyway?

Performance of course. Qemu today can advertise physical block size and
logical block size greater then 512 with virtio. The problem is that if
logical block size != 512 boot fails.

[Sebastian Herbszt]

Gleb Natapov wrote:
> On Sat, May 15, 2010 at 10:08:54PM +0200, Sebastian Herbszt wrote:
> > Why is a 4 KB physical sector size needed in a VM anyway?
>
> Performance of course. Qemu today can advertise physical block size and
> logical block size greater then 512 with virtio. The problem is that if
> logical block size != 512 boot fails.

Which OS currently has native support for 4 KB sectors and how big is the
performance gain? The performance without native support (emulation in
BIOS) could be worse due to the additional overhead. How much performance
is lost if the 512 byte logical sector size emulation is done in virtio (for reads
it could behave like a read ahead buffer) ?

Sebastian

[Gleb Natapov]

On Sun, May 16, 2010 at 03:34:23PM +0200, Sebastian Herbszt wrote:
> Gleb Natapov wrote:
> > On Sat, May 15, 2010 at 10:08:54PM +0200, Sebastian Herbszt wrote:
> > > Why is a 4 KB physical sector size needed in a VM anyway?
> >
> > Performance of course. Qemu today can advertise physical block size and
> > logical block size greater then 512 with virtio. The problem is that if
> > logical block size != 512 boot fails.
>
> Which OS currently has native support for 4 KB sectors and how big is the
> performance gain?

I believe at least Linux has it. Windows 7 aligns partition to 4K, so
it either has it too, or ready to gain it any moment.
>               The performance without native support (emulation in 
>
> BIOS) could be worse due to the additional overhead. How much performance
> is lost if the 512 byte logical sector size emulation is done in virtio (for reads
> it could behave like a read ahead buffer) ?

Low performance of BIOS emulation is not a concern. BIOS is used only by
boot loaders. OSes access disk without BIOS help.

[Sebastian Herbszt]

Gleb Natapov wrote:
> On Sun, May 16, 2010 at 03:34:23PM +0200, Sebastian Herbszt wrote:
> > Gleb Natapov wrote:
> > > On Sat, May 15, 2010 at 10:08:54PM +0200, Sebastian Herbszt wrote:
> > > > Why is a 4 KB physical sector size needed in a VM anyway?
> > >
> > > Performance of course. Qemu today can advertise physical block size and
> > > logical block size greater then 512 with virtio. The problem is that if
> > > logical block size != 512 boot fails.
> >
> > Which OS currently has native support for 4 KB sectors and how big is the
> > performance gain?
>
> I believe at least Linux has it. Windows 7 aligns partition to 4K, so
> it either has it too, or ready to gain it any moment.

According to KB 923332 [1] Vista started to align partitions:

"In earlier versions of Windows, the default starting offset for the first partition on a
hard disk drive was sector 0x3F. Because this starting offset was an odd number, it
could cause performance issues on large-sector drives because of misalignment between
the partition and the physical sectors. In Windows Vista, the default starting offset will
generally be sector 0x800."

Sector 0x800 with 512 byte logical sectors is 1 MB. With 4 KB logical sectors it would
be 8 MB.
> >               The performance without native support (emulation in 
> >
> > BIOS) could be worse due to the additional overhead. How much performance
> > is lost if the 512 byte logical sector size emulation is done in virtio (for reads
> > it could behave like a read ahead buffer) ?
>
> Low performance of BIOS emulation is not a concern. BIOS is used only by
> boot loaders. OSes access disk without BIOS help.

Legacy OS or OS installers tend to use BIOS disk access too.

[1] http://support.microsoft.com/kb/923332/EN-US/

Sebastian

[Gleb Natapov]

On Sun, May 16, 2010 at 06:14:57PM +0200, Sebastian Herbszt wrote:
> > >              The performance without native support
> > >
> > > (emulation in BIOS) could be worse due to the additional
> > > overhead. How much performance
> > > is lost if the 512 byte logical sector size emulation is done in virtio (for reads
> > > it could behave like a read ahead buffer) ?
> >
> > Low performance of BIOS emulation is not a concern. BIOS is used only by
> > boot loaders. OSes access disk without BIOS help.
>
> Legacy OS or OS installers tend to use BIOS disk access too.

So what choice legacy OS has when faced with drives with 4K logical
block size? Either it will not work, or will works slightly slower. (I
doubt that the slowdown will be actually measurable).

[Gleb Natapov]

On Sat, May 15, 2010 at 05:17:31PM +0200, Sebastian Herbszt wrote:
> Gleb Natapov wrote:
> > On Wed, May 12, 2010 at 07:25:55PM -0400, Kevin O'Connor wrote:
> > > The blksize needs to be 512, because the BIOS interface that virtio
> > > binds to requires 512 byte sectors.  Shouldn't this look something
> > > like:
> > >      if (cfg.blk_size != DISK_SECTOR_SIZE)
> > >          goto fail;
> > >      vdrive_g->drive.blksize = DISK_SECTOR_SIZE;
> >
> > Hmm, I guess you are right. Boot indeed fails if I configure logical
> > block size bigger then 512 bytes. But shouldn't BIOS emulate 512 byte
> > access on top of bigger block size? Future disks will use much large
> > logical block sizes.
>
> What the BIOS should do or not doesn't seem to be defined anywhere (yet).
> Current drives with physical sector size > 512 bytes report a logical sector
> size of 512 bytes and do the emulation on their own. The BIOS doesn't need
> to be modified so far to support those drives. If drives with logical sector
> size > 512 bytes appear this situation might change. Doing a read emulation

Isn't it chicken n' egg situation. If BIOSes will not add 4K block support
drives will never be able to use 4K logical blocks.
> in BIOS shouldn't be too bad, but a write emulation will need a "physical
> sector size" RMW buffer (no clue where the BIOS could steal those 4KB from).
> Maybe we are all using EFI without legacy BIOS interfaces when those drives
> appear anyway.
>
> Sebastian

[Kevin O'Connor]

On Sun, May 16, 2010 at 11:08:57AM +0300, Gleb Natapov wrote:
> On Sat, May 15, 2010 at 05:17:31PM +0200, Sebastian Herbszt wrote:
> > What the BIOS should do or not doesn't seem to be defined anywhere (yet).
> > Current drives with physical sector size > 512 bytes report a logical sector
> > size of 512 bytes and do the emulation on their own. The BIOS doesn't need
> > to be modified so far to support those drives. If drives with logical sector
> > size > 512 bytes appear this situation might change. Doing a read emulation
>
> Isn't it chicken n' egg situation. If BIOSes will not add 4K block support
> drives will never be able to use 4K logical blocks.

It was my understanding that new drives will start up in an emulation
mode which uses 512 byte sectors.  Later on the OS can activate the
native sector size.

This also raises a possibility for virtio - maybe a new command or
flag could be added to temporarily support 512 byte reads.  Thus the
main OS wont be penalized, and the bios wont have to emulate the reads
(the host will do it instead).  That said, I'm not against a patch
which generalizes the SeaBIOS cdemu block conversion code for virtio
if you wish to do that.  (It will reserve more memory, but I doubt
anyone using virtio will care.)

-Kevin

[Sebastian Herbszt]

Kevin O'Connor wrote:
> On Sun, May 16, 2010 at 11:08:57AM +0300, Gleb Natapov wrote:
> > On Sat, May 15, 2010 at 05:17:31PM +0200, Sebastian Herbszt wrote:
> > > What the BIOS should do or not doesn't seem to be defined anywhere (yet).
> > > Current drives with physical sector size > 512 bytes report a logical sector
> > > size of 512 bytes and do the emulation on their own. The BIOS doesn't need
> > > to be modified so far to support those drives. If drives with logical sector
> > > size > 512 bytes appear this situation might change. Doing a read emulation
> >
> > Isn't it chicken n' egg situation. If BIOSes will not add 4K block support
> > drives will never be able to use 4K logical blocks.
>
> It was my understanding that new drives will start up in an emulation
> mode which uses 512 byte sectors.  Later on the OS can activate the
> native sector size.

AFAIK that "activate" doesn't exist. It starts in emulation mode and stays there.
I was suggesting something like that "activate" in my other reply, but i don't think
that's available (yet).

Sebastian
> This also raises a possibility for virtio - maybe a new command or
> flag could be added to temporarily support 512 byte reads.  Thus the
> main OS wont be penalized, and the bios wont have to emulate the reads
> (the host will do it instead).  That said, I'm not against a patch
> which generalizes the SeaBIOS cdemu block conversion code for virtio
> if you wish to do that.  (It will reserve more memory, but I doubt
> anyone using virtio will care.)
>
> -Kevin

[Gleb Natapov]

Adding Christoph to CC who know much more then me about how block layer
works in qemu and linux.

On Sun, May 16, 2010 at 11:17:42AM -0400, Kevin O'Connor wrote:
> On Sun, May 16, 2010 at 11:08:57AM +0300, Gleb Natapov wrote:
> > On Sat, May 15, 2010 at 05:17:31PM +0200, Sebastian Herbszt wrote:
> > > What the BIOS should do or not doesn't seem to be defined anywhere (yet).
> > > Current drives with physical sector size > 512 bytes report a logical sector
> > > size of 512 bytes and do the emulation on their own. The BIOS doesn't need
> > > to be modified so far to support those drives. If drives with logical sector
> > > size > 512 bytes appear this situation might change. Doing a read emulation
> >
> > Isn't it chicken n' egg situation. If BIOSes will not add 4K block support
> > drives will never be able to use 4K logical blocks.
>
> It was my understanding that new drives will start up in an emulation
> mode which uses 512 byte sectors.  Later on the OS can activate the
> native sector size.

I am not sure disk can change its topology like that. The meaning of LBA
will change depending on what mode the disk is currently in.
> This also raises a possibility for virtio - maybe a new command or
> flag could be added to temporarily support 512 byte reads.  Thus the
> main OS wont be penalized, and the bios wont have to emulate the reads
> (the host will do it instead).  That said, I'm not against a patch
> which generalizes the SeaBIOS cdemu block conversion code for virtio
> if you wish to do that.  (It will reserve more memory, but I doubt
> anyone using virtio will care.)

As far as I understand if physical disk qemu image resides on has 4K
logical block size we cannot access it from inside a guest using 512 byte
i/o (disk image is opened with O_DIRECT and each i/o has to be
sector size aligned). So even if theoretically emulating 512 byte access
on top of 4K physical disk sector size inside virtio-blk in qemu is possible
it looks more complicated then just switching logical block size during
VM runtime. Christoph what do you think?