Re: Clarification regarding new qemu-img convert --target-is-zero flag
On Wed, Jun 10, 2020 at 7:31 PM Kevin Wolf wrote: > Am 10.06.2020 um 17:26 hat Sam Eiderman geschrieben: > > Thanks for the clarification Kevin, > > > > Well first I want to discuss unallocated blocks. > > From my understanding operating systems do not rely on disks to be > > zero initialized, on the contrary, physical disks usually contain > > garbage. > > So an unallocated block should never be treated as zero by any real > > world application. > > I think this is a dangerous assumption to make. The guest did have > access to these unallocated blocks before, and they read as zero, so not > writing these to the conversion target does change the virtual disk. > Whether or not this is a harmless change for the guest depends on the > software running in the VM. > I see your point > > > Now assuming that I only care about the allocated content of the > > disks, I would like to save io/time zeroing out unallocated blocks. > > > > A real world example would be flushing a 500GB vmdk on a real SSD > > disk, if the vmdk contained only 2GB of data, no point in writing > > 498GB of zeroes to that SSD - reducing its lifespan for nothing. > > Don't pretty much all SSDs support efficient zeroing/hole punching these > days so that the blocks would actually be deallocated on the disk level? > > > Now from what I understand --target-is-zero will give me this behavior > > even though that I really use it as a "--skip-prezeroing-target" > > (sorry for the bad name) > > (This is only true if later *allocated zeroes* are indeed copied > correctly) > > As you noticed later, it doesn't. > > The behaviour you want is more like -B, except that you don't have a > backing file. If you also pass -n, the actual filename you pass isn't > even used, so I guess '-B "" -n' should do the trick? > > Kevin > >
Re: Clarification regarding new qemu-img convert --target-is-zero flag
Ok great, thanks for making it clear. On Wed, Jun 10, 2020 at 6:42 PM David Edmondson wrote: > > On Wednesday, 2020-06-10 at 18:29:33 +03, Sam Eiderman wrote: > > > Excuse me, > > > > Vladimir already pointed out in the first comment that it will skip > > writing real zeroes later. > > Right. That's why you want something like "--no-need-to-zero-initialise" > (the name keeps getting longer!), which would still write zeroes to the > blocks that should contain zeroes, as opposed to writing zeroes to > prepare the device. > > > Sam > > > > On Wed, Jun 10, 2020 at 6:26 PM Sam Eiderman wrote: > >> > >> Thanks for the clarification Kevin, > >> > >> Well first I want to discuss unallocated blocks. > >> From my understanding operating systems do not rely on disks to be > >> zero initialized, on the contrary, physical disks usually contain > >> garbage. > >> So an unallocated block should never be treated as zero by any real > >> world application. > >> > >> Now assuming that I only care about the allocated content of the > >> disks, I would like to save io/time zeroing out unallocated blocks. > >> > >> A real world example would be flushing a 500GB vmdk on a real SSD > >> disk, if the vmdk contained only 2GB of data, no point in writing > >> 498GB of zeroes to that SSD - reducing its lifespan for nothing. > >> > >> Now from what I understand --target-is-zero will give me this behavior > >> even though that I really use it as a "--skip-prezeroing-target" > >> (sorry for the bad name) > >> (This is only true if later *allocated zeroes* are indeed copied correctly) > >> > >> Sam > >> > >> On Wed, Jun 10, 2020 at 5:06 PM Kevin Wolf wrote: > >> > > >> > Am 10.06.2020 um 14:19 hat Sam Eiderman geschrieben: > >> > > Thanks David, > >> > > > >> > > Yes, I imaging the following use case: > >> > > > >> > > disk.vmdk is a 50 GB disk that contains 12 MB binary of zeroes in its > >> > > beginning. > >> > > /dev/sda is a raw disk containing garbage > >> > > > >> > > I invoke: > >> > > qemu-img convert disk.vmdk -O raw /dev/sda > >> > > > >> > > Required output: > >> > > The first 12 MB of /dev/sda contain zeros, the rest garbage, qemu-img > >> > > finishes fast. > >> > > > >> > > Kevin, from what I understood from you, this is the default behavior. > >> > > >> > Sorry, I misunderstood what you want. qemu-img will write zeros to all > >> > unallocated parts, too. If it didn't do that, the resulting image on > >> > /dev/sda wouldn't be a copy of disk.vmdk. > >> > > >> > As the metadata (which blocks are allocated) cannot be preserved in raw > >> > images, you wouldn't be able to tell which part of the image contains > >> > valid data and which part needs to be interpreted as zeros even though > >> > it contains random garbage. > >> > > >> > What is your use case for this result where the actual virtual disk > >> > content is mixed with garbage? > >> > > >> > Kevin > >> > > > dme. > -- > He caught a fleeting glimpse of a man, moving uphill pursued by a bus.
Re: Clarification regarding new qemu-img convert --target-is-zero flag
Excuse me, Vladimir already pointed out in the first comment that it will skip writing real zeroes later. Sam On Wed, Jun 10, 2020 at 6:26 PM Sam Eiderman wrote: > > Thanks for the clarification Kevin, > > Well first I want to discuss unallocated blocks. > From my understanding operating systems do not rely on disks to be > zero initialized, on the contrary, physical disks usually contain > garbage. > So an unallocated block should never be treated as zero by any real > world application. > > Now assuming that I only care about the allocated content of the > disks, I would like to save io/time zeroing out unallocated blocks. > > A real world example would be flushing a 500GB vmdk on a real SSD > disk, if the vmdk contained only 2GB of data, no point in writing > 498GB of zeroes to that SSD - reducing its lifespan for nothing. > > Now from what I understand --target-is-zero will give me this behavior > even though that I really use it as a "--skip-prezeroing-target" > (sorry for the bad name) > (This is only true if later *allocated zeroes* are indeed copied correctly) > > Sam > > On Wed, Jun 10, 2020 at 5:06 PM Kevin Wolf wrote: > > > > Am 10.06.2020 um 14:19 hat Sam Eiderman geschrieben: > > > Thanks David, > > > > > > Yes, I imaging the following use case: > > > > > > disk.vmdk is a 50 GB disk that contains 12 MB binary of zeroes in its > > > beginning. > > > /dev/sda is a raw disk containing garbage > > > > > > I invoke: > > > qemu-img convert disk.vmdk -O raw /dev/sda > > > > > > Required output: > > > The first 12 MB of /dev/sda contain zeros, the rest garbage, qemu-img > > > finishes fast. > > > > > > Kevin, from what I understood from you, this is the default behavior. > > > > Sorry, I misunderstood what you want. qemu-img will write zeros to all > > unallocated parts, too. If it didn't do that, the resulting image on > > /dev/sda wouldn't be a copy of disk.vmdk. > > > > As the metadata (which blocks are allocated) cannot be preserved in raw > > images, you wouldn't be able to tell which part of the image contains > > valid data and which part needs to be interpreted as zeros even though > > it contains random garbage. > > > > What is your use case for this result where the actual virtual disk > > content is mixed with garbage? > > > > Kevin > >
Re: Clarification regarding new qemu-img convert --target-is-zero flag
Thanks for the clarification Kevin, Well first I want to discuss unallocated blocks. >From my understanding operating systems do not rely on disks to be zero initialized, on the contrary, physical disks usually contain garbage. So an unallocated block should never be treated as zero by any real world application. Now assuming that I only care about the allocated content of the disks, I would like to save io/time zeroing out unallocated blocks. A real world example would be flushing a 500GB vmdk on a real SSD disk, if the vmdk contained only 2GB of data, no point in writing 498GB of zeroes to that SSD - reducing its lifespan for nothing. Now from what I understand --target-is-zero will give me this behavior even though that I really use it as a "--skip-prezeroing-target" (sorry for the bad name) (This is only true if later *allocated zeroes* are indeed copied correctly) Sam On Wed, Jun 10, 2020 at 5:06 PM Kevin Wolf wrote: > > Am 10.06.2020 um 14:19 hat Sam Eiderman geschrieben: > > Thanks David, > > > > Yes, I imaging the following use case: > > > > disk.vmdk is a 50 GB disk that contains 12 MB binary of zeroes in its > > beginning. > > /dev/sda is a raw disk containing garbage > > > > I invoke: > > qemu-img convert disk.vmdk -O raw /dev/sda > > > > Required output: > > The first 12 MB of /dev/sda contain zeros, the rest garbage, qemu-img > > finishes fast. > > > > Kevin, from what I understood from you, this is the default behavior. > > Sorry, I misunderstood what you want. qemu-img will write zeros to all > unallocated parts, too. If it didn't do that, the resulting image on > /dev/sda wouldn't be a copy of disk.vmdk. > > As the metadata (which blocks are allocated) cannot be preserved in raw > images, you wouldn't be able to tell which part of the image contains > valid data and which part needs to be interpreted as zeros even though > it contains random garbage. > > What is your use case for this result where the actual virtual disk > content is mixed with garbage? > > Kevin >
Re: Clarification regarding new qemu-img convert --target-is-zero flag
Thanks David,
Yes, I imaging the following use case:
disk.vmdk is a 50 GB disk that contains 12 MB binary of zeroes in its beginning.
/dev/sda is a raw disk containing garbage
I invoke:
qemu-img convert disk.vmdk -O raw /dev/sda
Required output:
The first 12 MB of /dev/sda contain zeros, the rest garbage, qemu-img
finishes fast.
Kevin, from what I understood from you, this is the default behavior.
So if my VMDK is causing trouble (all virtual size is being written)
this is probably since all the grains in the VMDK are zero allocated
right?
Thanks!
On Wed, Jun 10, 2020 at 2:56 PM David Edmondson wrote:
>
> On Wednesday, 2020-06-10 at 08:28:29 +03, Sam Eiderman wrote:
>
> > Hi,
> >
> > 168468fe19c8 ("qemu-img: Add --target-is-zero to convert") has added a
> > nice functionality for cloud scenarios:
> >
> > * Create a virtual disk
> > * Convert a sparse image (qcow2, vmdk) to the virtual disk using
> > --target-is-zero
> > * Use the virtual disk
> >
> > This saves many unnecessary writes - a qcow2 with 1MB of allocated
> > data but with 100GB virtual size will be converted efficiently.
> >
> > However, does this pose a problem if the virtual disk is not zero
> > initialized?
>
> As Vladimir indicated, the intent of the flag is supposed to be clear
> from the name :-) If your storage doesn't read zeroes absent any earlier
> writes, you probably don't want to be using it.
>
> > Theoretically - if all unallocated blocks contain garbage - this
> > shouldn't matter, however what about allocated blocks of zero? Will
> > convert skip copying allocated zero blocks in the source image to the
> > target since it assumes that the target is zeroed out first thing?
>
> So something like a "--no-need-to-zero" flag would do what you want,
> presuming that it would write known zeroes but no longer clean the
> device before use?
>
> dme.
> --
> You can't hide from the flipside.
Re: Clarification regarding new qemu-img convert --target-is-zero flag
I see,
I thought qemu-img (by default) checks the virtual size of the disk
before starting to copy allocated data, zeroes out all of the virtual
size (slowly) and then writes all the allocated data except for
zeroes.
But from what I understand now, qemu-img finds that the target is raw
and can not be efficiently zeroed, so it just writes all the allocated
data, including zeroes, leaving unallocated gaps in the virtual size
unwritten.
I have an image of 800MB VMDK with virtual size of 24GB
So if the following:
qemu-img convert "${IMAGE_PATH}" -p -O raw -S 512b /dev/sdc 2>&1
Takes roughly 3 minutes and 40 seconds (qemu 3.1.0)
And:
qemu-img convert "${IMAGE_PATH}" -n --target-is-zero -p -O raw /dev/sdc 2>&1
Takes roughly 2 seconds (qemu 5.0.0)
This means that probably there are ~23GB of zeroes *allocated* in this VMDK,
I'll check that.
Sam
On Wed, Jun 10, 2020 at 2:37 PM Kevin Wolf wrote:
>
> Am 10.06.2020 um 08:28 hat Sam Eiderman geschrieben:
> > Hi,
> >
> > My target format is a Persistent Disk on GCP.
> > https://cloud.google.com/persistent-disk
> >
> > And my use case is converting VMDKs to PDs so I'm just using qemu-img
> > for the conversion (not using qemu as a hypervisor).
> >
> > Luckily PDs are zeroed out when allocated but I was asking to
> > understand the restrictions of qemu-img convert.
> >
> > It could be useful for qemu-img convert to not zero out the disk, but
> > do write allocated zeroes, I'm imagining cloud scenarios where instead
> > of virtual disks the customer receives an attached physical SSD device
> > that is not zeroed out beforehand (only encryption key changed, for
> > privacy/security sake) so reads will return garbage.
>
> But that's the default mode? Zeroing out the whole disk upfront is an
> optimisation that we do if efficient zeroing is possible, but if we
> can't, we just write explicit zeros where needed.
>
> --target-is-zero means that you promise that the target is already
> pre-zeroed so qemu-img can further optimise things. If you specify it
> and the target doesn't contain zeros, but random data, you get garbage.
>
> Kevin
>
Re: Clarification regarding new qemu-img convert --target-is-zero flag
Hi,
My target format is a Persistent Disk on GCP.
https://cloud.google.com/persistent-disk
And my use case is converting VMDKs to PDs so I'm just using qemu-img
for the conversion (not using qemu as a hypervisor).
Luckily PDs are zeroed out when allocated but I was asking to
understand the restrictions of qemu-img convert.
It could be useful for qemu-img convert to not zero out the disk, but
do write allocated zeroes, I'm imagining cloud scenarios where instead
of virtual disks the customer receives an attached physical SSD device
that is not zeroed out beforehand (only encryption key changed, for
privacy/security sake) so reads will return garbage.
Sam
On Wed, Jun 10, 2020 at 9:16 AM Vladimir Sementsov-Ogievskiy
wrote:
>
> Hi Sam!
>
> 10.06.2020 08:28, Sam Eiderman wrote:
> > Hi,
> >
> > 168468fe19c8 ("qemu-img: Add --target-is-zero to convert") has added a
> > nice functionality for cloud scenarios:
> >
> > * Create a virtual disk
>
> What is the format of your target?
>
> > * Convert a sparse image (qcow2, vmdk) to the virtual disk using
> > --target-is-zero
> > * Use the virtual disk
> >
> > This saves many unnecessary writes - a qcow2 with 1MB of allocated
> > data but with 100GB virtual size will be converted efficiently.
> >
> > However, does this pose a problem if the virtual disk is not zero
> > initialized?
> >
> > Theoretically - if all unallocated blocks contain garbage - this
> > shouldn't matter, however what about allocated blocks of zero? Will
> > convert skip copying allocated zero blocks in the source image to the
> > target since it assumes that the target is zeroed out first thing?
> >
>
> Yes, the feature is only for really zero-initialized target, it will skip
> "allocated" zeroes as well.
>
> What you want - copying only allocated blocks of backing-supporting format -
> looks like "top" mode of
> mirror and backup block jobs. Didn't you considered using qemu itself (in
> stopped mode, i.e. cpus are
> not running) or new qemu-storage-daemon instead of qemu-img? With this
> approach you'll have the whole
> power of QMP commands to manage block-layer, including block-jobs.
>
> --
> Best regards,
> Vladimir
Clarification regarding new qemu-img convert --target-is-zero flag
Hi,
168468fe19c8 ("qemu-img: Add --target-is-zero to convert") has added a
nice functionality for cloud scenarios:
* Create a virtual disk
* Convert a sparse image (qcow2, vmdk) to the virtual disk using
--target-is-zero
* Use the virtual disk
This saves many unnecessary writes - a qcow2 with 1MB of allocated
data but with 100GB virtual size will be converted efficiently.
However, does this pose a problem if the virtual disk is not zero initialized?
Theoretically - if all unallocated blocks contain garbage - this
shouldn't matter, however what about allocated blocks of zero? Will
convert skip copying allocated zero blocks in the source image to the
target since it assumes that the target is zeroed out first thing?
Sam
Re: gitlab-ci: Do not use the standard container images from gitlab
I see, thanks for the clarification. However sometimes builds usually do tend to work on Ubuntu but fail to work on Debian since it's not always a 1-1 (as in this case) - so you might want to consider to keep testing Debian together with Ubuntu. Regarding the Ubuntu 20 problem - have you tried "export DEBIAN_FRONTEND=noninteractive"? didn't see it in the logs Sam On Sun, Jun 7, 2020 at 8:39 AM Thomas Huth wrote: > > On 06/06/2020 14.38, Sam Eiderman wrote: > > Thanks for the link > > > > I do believe that the correct approach for me is to rename > > BITS_PER_LONG to __BITS_PER_LONG (I just added a sed command in my > > Dockerfile) and move on with my particular usage, however I am just > > wondering whether dropping debian10/ubuntu20 in the official qemu ci/ > > pipeline until it's fixed is the correct approach instead of keep > > failing it until the error resolves, in a way we want to always know > > on which OSs the compilation fails for visibility, no? > > Hi, > > that bug was only one reason to move the pipelines to another OS. The > other reason is that we are already extensively testing various Ubuntu > (and thus Debian-based) versions in the Travis CI - but did not test any > RPM-based distros in the CI yet. Since Travis is bound to Ubuntu, we can > not test Fedora/CentOS there, thus the Gitlab CI pipelines have now been > moved to RPM-based distros (except for the "build-user" pipeline which > is still using Debian, and the "build-system1" which is now using Ubuntu > 19.04 instead, so I think we still have a good mix there). > > Note that the problem with Ubuntu 20.04 is also something completely > different: It hangs in an interactive prompt during update and waits for > user input, so that the pipelines finally times out: > > https://gitlab.com/huth/qemu/-/jobs/584573287#L800 > > If you know a work-around for that, we can move the build-system1 > pipeline from 19.04 to 20.04 ... or if Debian gets finally fixed, we can > also move that pipeline back to Debian. I'm fine either way, as long as > the pipelines do not fail due to non-QEMU bugs in the distros. > > Thomas >
Re: gitlab-ci: Do not use the standard container images from gitlab
Thanks for the link I do believe that the correct approach for me is to rename BITS_PER_LONG to __BITS_PER_LONG (I just added a sed command in my Dockerfile) and move on with my particular usage, however I am just wondering whether dropping debian10/ubuntu20 in the official qemu ci/ pipeline until it's fixed is the correct approach instead of keep failing it until the error resolves, in a way we want to always know on which OSs the compilation fails for visibility, no? Thanks again! On Sat, Jun 6, 2020 at 2:49 PM Alex Bennée wrote: > > > Sam Eiderman writes: > > > Hi, > > > > I am using debian 10 container to compile qemu too. > > > > I think that what happens here is that > > > > /usr/include/linux/swab.h > > > > Uses BITS_PER_LONG instead of __BITS_PER_LONG which is actually defined > > before > > in qemu at: > > That is indeed the error - we are just waiting for Debian to update > linux-libc-dev with the fix to the kernel headers: > > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=960271 > > > > > include/qemu/bitops.h:#define BITS_PER_LONG (sizeof (unsigned > > long) * BITS_PER_BYTE) > > > > which injects this definition into the linux swab.h header. > > > > By changing BITS_PER_LONG to __BITS_PER_LONG in the linux headers, I > > managed to > > successfully compile qemu. > > > > A different approach would be to move the linux header includes > > (#include ) in file-posix.c above all other includes - which > > in > > some way makes more sense (since we probaly don't want qemu defines to > > control > > linux headers) but it requires a more complex refactoring. > > > -- > Alex Bennée
gitlab-ci: Do not use the standard container images from gitlab
Hi, I am using debian 10 container to compile qemu too. I think that what happens here is that /usr/include/linux/swab.h Uses BITS_PER_LONG instead of __BITS_PER_LONG which is actually defined before in qemu at: include/qemu/bitops.h:#define BITS_PER_LONG (sizeof (unsigned long) * BITS_PER_BYTE) which injects this definition into the linux swab.h header. By changing BITS_PER_LONG to __BITS_PER_LONG in the linux headers, I managed to successfully compile qemu. A different approach would be to move the linux header includes (#include ) in file-posix.c above all other includes - which in some way makes more sense (since we probaly don't want qemu defines to control linux headers) but it requires a more complex refactoring.
Re: Regarding SESparse support in QEMU
Hi, This is regarding the new VMDK format of SESparse snapshots which is available since ESXi 6.5 (for disks > 2TB) and is default since ESXi 6.7 (for all disks). Unlike the previous format (VMFSSparse), SESparse's format is not disclosed by VMware. Even though, I believe that the format itself is not too complicated and I'll try to give out pointers on how it is possible to implement a read-write support for it. In commit 98eb9733f (vmdk: Add read-only support for seSparse snapshots) I have added a read-only support. As stated in the commit message - I did not implement the following features which are implemented in VMware: * read-write * journal replay * space reclamation * unmap support I don't fully understand what you're trying to implement when you say "read-write" since there are some scenarios for which you will not need to implement a fully compatible SESparse image. After creating an SESparse snapshot (by invoking "Take Snapshot" in VMware), the following file: /var/log/hostd.log Contains messages of the like: [...] Const Header: [...] constMagic = 0xcafebabe [...] version= 2.1 [...] capacity = 204800 [...] grainSize = 8 [...] grainTableSize = 64 [...] flags = 0 [...] Extents: [...] Header : <1 : 1> [...] JournalHdr : <2 : 2> [...] Journal: <2048 : 2048> [...] GrainDirectory : <4096 : 2048> [...] GrainTables: <6144 : 2048> [...] FreeBitmap : <8192 : 2048> [...] BackMap: <10240 : 2048> [...] Grain : <12288 : 204800> [...] Volatile Header: [...] volatileMagic = 0xcafecafe [...] FreeGTNumber = 0 [...] nextTxnSeqNumber = 0 [...] replayJournal = 0 The sizes that are seen in the log file are in sectors. Extents are of the following format: Just from the strings in this message and by information publicly disclosed by VMware it is obvious how SESparse is implemented: The problem with VMFSSparse (previous format) is that given an offset to a grain in the file - you don't know if that grain is allocated or not - you must scan all grain tables to perform space reclamation, and even if somehow you know that the grain is not allocated - you don't have the offset to the grain table that references it - to unmap it from there. This was obviously solved using the structures "FreeBitmap" and "BackMap". FreeBitmap is simply a very big bitmap that tells you if the grain at the offset of the bit is allocated. Meaning: If bit X is set to 1 then Grain[X] is allocated (Notice that each grain is of the size "grainSize" sectors - usually 4K bytes). Now, we can simply iterate through FreeBitmap to find grains we can reclaim. Problem - we don't know where is the grain table to update, scanning all grain tables is slow. Solution: BackMap: For Grain[X] look at BackMap[X] - BackMap[X] contains an index to GrainTables at which the grain table resides (Multiply by the size of a grain table in order to get the real offset). (Notice that BackMap scales with the size of the VMDK - but only 8 bytes per entry instead 4K bytes) Problem - when we now write to BackMap & FreeBitmap structures too - we might experience data corruption of the SESparse format. Solution: Journal Write data (write what where) to the journal, mark journal dirty, execute instructions in journal, clean dirty mark - if you crash in the middle - you will reexecute the instructions when opening the VMDK file. (Notice this was not a problem in VMFSSprase since we could first write the data in the grain, then update the grain table to the grain, then update the grain directory to the grain table - so the data is actually written on the last operation and the format structure is not corrupted) In any case, now we need to implement the following: When opening the file: * Execute journal (not implemented) When reading * Follow Grain Directory -> Grain Table -> Grain (This is implemented now) For writing (not implemented, probably something like this) * Follow Grain Directory * If grain table allocated -> get it * Otherwise, allocate new grain table (this probably uses FreeGTNumber in the volatile header to know where to do so) * Extend the file by the size needed to store all the grains in the grain table (I think this is 16MB by default) * Follow Grain Table * If grain allocated, get it * Otherwise, allocate it * Write to grain (you may need to read it if you write only to some of it, since now grains are 4K - it is possible to read/write half a grain) * Update BackMap and FreeBitmap accordingly. * All of the above writes should go through the Journal for consistency. When unmapping: * Update bits in FreeBitmap (1 -> 0) When reclaiming space: Go through FreeBitmaps, decide if should perform cleaning (must have a complete empty grain table), if so, reclaim (You can only clean when a full Grain table is unmapped, copy another grain table in its stead, you'll have to update Grains, and the Grain d
[PULL 2/2] seabios: update to pre-1.13 snapshot again
Hi, Maybe we should add: CONFIG_HOST_BIOS_GEOMETRY=n to rom/config.seabios-128k and recreate the 128k image?
Re: [SeaBIOS] Re: [PATCH] ahci: zero-initialize port struct
Looks great Sam On Fri, Nov 15, 2019 at 1:35 PM Gerd Hoffmann wrote: > > Hi, > > > > I am afraid it will be slightly unreadable when looking at file histories. > > > The only commit that didn't change was: > > > [SeaBIOS] [PATCH v4 2/5] boot: Reorder functions in boot.c > > > > Hmm, looks like there have been more changes than I remember. > > Yep, v3..v4 diff is pretty big, so revert + apply v4 looks more > reasonable. Can you have a look and double-check? > > https://git.kraxel.org/cgit/seabios/log/?h=lchs-fixup > > thanks, > Gerd >
Re: [SeaBIOS] Re: [PATCH] ahci: zero-initialize port struct
Do you want a single commit with the changes? I am afraid it will be slightly unreadable when looking at file histories. The only commit that didn't change was: [SeaBIOS] [PATCH v4 2/5] boot: Reorder functions in boot.c Sam On Thu, Nov 14, 2019 at 9:20 AM Gerd Hoffmann wrote: > > On Wed, Nov 13, 2019 at 05:03:58PM +0200, Sam Eiderman wrote: > > Hi, > > > > Does this fix a bug that actually happened? > > Yes, "make check-qtest" may fail. It's kind of random though. > > > I just noticed that in my lchs patches I assumed that lchs struct is > > zeroed out in all devices (not only ahci): > > ahci was the only one not zeroing out the struct (yes, I've reviewed > them all). > > > Also Gerd it seems that my lchs patches were not committed in the > > latest submitted version (v4)!!! > > Whoops. Can you sent a patch seabios/master ... v4 please? > > IIRC there didn't change much, mostly the parser function, so that > should be alot less churn than a full revert + v4 reapply. > > thanks, > Gerd >
Re: [SeaBIOS] Re: [PATCH] ahci: zero-initialize port struct
Links to latest commits from archive. You can see all changes in the cover letter. [SeaBIOS] [PATCH v4 0/5] Add Qemu to SeaBIOS LCHS interface https://mail.coreboot.org/hyperkitty/list/seab...@seabios.org/message/VLNFBEERTWLEUO6LM5BYLBNVIFCTP46M/ [SeaBIOS] [PATCH v4 1/5] geometry: Read LCHS from fw_cfg https://mail.coreboot.org/hyperkitty/list/seab...@seabios.org/message/B3IPD3HH4UPDYJWFE4KX3HXUCNW5GPEW/ [SeaBIOS] [PATCH v4 2/5] boot: Reorder functions in boot.c https://mail.coreboot.org/hyperkitty/list/seab...@seabios.org/message/YDVU3WIGOSKZ2RQSMR5UVQNZ66K4IG65/ [SeaBIOS] [PATCH v4 3/5] boot: Build ata and scsi paths in function https://mail.coreboot.org/hyperkitty/list/seab...@seabios.org/message/RY33DRZZ3UK3UMQ3Q6BY2KUCHRRW4MRK/ [SeaBIOS] [PATCH v4 4/5] geometry: Add boot_lchs_find_*() utility functions https://mail.coreboot.org/hyperkitty/list/seab...@seabios.org/message/DAJOULWFK24DX4DY3VS6WWOOQNWW3GSG/ [SeaBIOS] [PATCH v4 5/5] geometry: Apply LCHS values for boot devices https://mail.coreboot.org/hyperkitty/list/seab...@seabios.org/message/UUCTPPJ4PTS5CUTCFLOH3YOEXGC6HQ4T/ Sam On Wed, Nov 13, 2019 at 6:35 PM Sam Eiderman wrote: > > Sure, > > There are two issues here. > > The first issue is that my commits which applied to seabios master: > > * 9caa19b - geometry: Apply LCHS values for boot devices > * cb56f61 - config: Add toggle for bootdevice information > * ad29109 - geometry: Add boot_lchs_find_*() utility functions > * b3d2120 - boot: Reorder functions in boot.c > * 7c66a43 - geometry: Read LCHS from fw_cfg > > Are not from the latest version which was submitted to the mailing list (v4) > * fw_cfg key name has changed > * The value and of the key has changed from binary (v1) to textual (v4) > * Other fixes and variable name changes. > > So these commits need to be reverted and reapplied with the latest version > (v4) > > The second issue is that my commits, (in v4 too) will require this fix > that Gerd added ([PATCH] ahci: zero-initialize port struct) since they > change how SeaBIOS uses lchs. > > Previously, before any of my commits, drive.lchs could contain "random > crap" since it was always set before being used in > setup_translation(). > > After my patches, get_translation() invokes overriden_lchs_supplied() > which checks: "return drive->lchs.cylinder || drive->lchs.head || > drive->lchs.sector;" > So there is an assumption that "drive->lchs" is zeroed when lchs is > not supplied for the host. > > This was true for all devices using "drive->lchs" (all were memset to > 0) except ahci. > (I used 'git grep "drive_s * drive"' to find them all). > > So Gerd fix is indeed needed and then all devices are covered > (drive->lchs is memset to 0). > > Now only the first issue remains... > > Sam > > On Wed, Nov 13, 2019 at 6:12 PM Philippe Mathieu-Daudé > wrote: > > > > Hi Sam, > > > > On 11/13/19 4:03 PM, Sam Eiderman wrote: > > > Hi, > > > > > > Does this fix a bug that actually happened? > > > > > > I just noticed that in my lchs patches I assumed that lchs struct is > > > zeroed out in all devices (not only ahci): > > > > > > 9caa19be0e53 (geometry: Apply LCHS values for boot devices) > > > > > > Seems like this is not the case but why only ahci is affected? > > > > > > The list of devices is at least: > > > > > > * ata > > > * ahci > > > * scsi > > > * esp > > > * lsi > > > * megasas > > > * mpt > > > * pvscsi > > > * virtio > > > * virtio-blk > > > > > > As specified in the commit message. > > > > > > Also Gerd it seems that my lchs patches were not committed in the > > > latest submitted version (v4)!!! > > > The ABI of the fw config key is completely broken. > > > > What do you mean? Can you be more specific? > >
Re: [SeaBIOS] Re: [PATCH] ahci: zero-initialize port struct
Sure, There are two issues here. The first issue is that my commits which applied to seabios master: * 9caa19b - geometry: Apply LCHS values for boot devices * cb56f61 - config: Add toggle for bootdevice information * ad29109 - geometry: Add boot_lchs_find_*() utility functions * b3d2120 - boot: Reorder functions in boot.c * 7c66a43 - geometry: Read LCHS from fw_cfg Are not from the latest version which was submitted to the mailing list (v4) * fw_cfg key name has changed * The value and of the key has changed from binary (v1) to textual (v4) * Other fixes and variable name changes. So these commits need to be reverted and reapplied with the latest version (v4) The second issue is that my commits, (in v4 too) will require this fix that Gerd added ([PATCH] ahci: zero-initialize port struct) since they change how SeaBIOS uses lchs. Previously, before any of my commits, drive.lchs could contain "random crap" since it was always set before being used in setup_translation(). After my patches, get_translation() invokes overriden_lchs_supplied() which checks: "return drive->lchs.cylinder || drive->lchs.head || drive->lchs.sector;" So there is an assumption that "drive->lchs" is zeroed when lchs is not supplied for the host. This was true for all devices using "drive->lchs" (all were memset to 0) except ahci. (I used 'git grep "drive_s * drive"' to find them all). So Gerd fix is indeed needed and then all devices are covered (drive->lchs is memset to 0). Now only the first issue remains... Sam On Wed, Nov 13, 2019 at 6:12 PM Philippe Mathieu-Daudé wrote: > > Hi Sam, > > On 11/13/19 4:03 PM, Sam Eiderman wrote: > > Hi, > > > > Does this fix a bug that actually happened? > > > > I just noticed that in my lchs patches I assumed that lchs struct is > > zeroed out in all devices (not only ahci): > > > > 9caa19be0e53 (geometry: Apply LCHS values for boot devices) > > > > Seems like this is not the case but why only ahci is affected? > > > > The list of devices is at least: > > > > * ata > > * ahci > > * scsi > > * esp > > * lsi > > * megasas > > * mpt > > * pvscsi > > * virtio > > * virtio-blk > > > > As specified in the commit message. > > > > Also Gerd it seems that my lchs patches were not committed in the > > latest submitted version (v4)!!! > > The ABI of the fw config key is completely broken. > > What do you mean? Can you be more specific? >
Re: [SeaBIOS] Re: [PATCH] ahci: zero-initialize port struct
Sorry the correct list (which includes ahci) is: git grep "drive_s * drive" src/hw/ahci.h:struct drive_s drive; src/hw/ata.h:struct drive_s drive; src/hw/esp-scsi.c:struct drive_s drive; src/hw/lsi-scsi.c:struct drive_s drive; src/hw/megasas.c:struct drive_s drive; src/hw/mpt-scsi.c:struct drive_s drive; src/hw/nvme-int.h:struct drive_s drive; src/hw/pvscsi.c:struct drive_s drive; src/hw/sdcard.c:struct drive_s drive; src/hw/usb-msc.c:struct drive_s drive; src/hw/usb-uas.c:struct drive_s drive; src/hw/virtio-blk.c:struct drive_s drive; src/hw/virtio-scsi.c:struct drive_s drive; Went over this list now, seems only ahci_port_alloc was missing memset(0) so after this patch all of the devices that contain lchs are covered. But we still need to revert my lchs patches and reapply them with the newest version (v4) Sam On Wed, Nov 13, 2019 at 5:18 PM Sam Eiderman wrote: > > We should make sure drive.lchs is zeroed out for the following devices: > > git grep "drive_s drive" > > src/hw/ata.h:struct drive_s drive; > src/hw/esp-scsi.c:struct drive_s drive; > src/hw/lsi-scsi.c:struct drive_s drive; > src/hw/megasas.c:struct drive_s drive; > src/hw/mpt-scsi.c:struct drive_s drive; > src/hw/nvme-int.h:struct drive_s drive; > src/hw/pvscsi.c:struct drive_s drive; > src/hw/sdcard.c:struct drive_s drive; > src/hw/usb-msc.c:struct drive_s drive; > src/hw/usb-uas.c:struct drive_s drive; > src/hw/virtio-blk.c:struct drive_s drive; > src/hw/virtio-scsi.c: struct drive_s drive; > > Sam > > > On Wed, Nov 13, 2019 at 5:03 PM Sam Eiderman wrote: > > > > Hi, > > > > Does this fix a bug that actually happened? > > > > I just noticed that in my lchs patches I assumed that lchs struct is > > zeroed out in all devices (not only ahci): > > > > 9caa19be0e53 (geometry: Apply LCHS values for boot devices) > > > > Seems like this is not the case but why only ahci is affected? > > > > The list of devices is at least: > > > > * ata > > * ahci > > * scsi > > * esp > > * lsi > > * megasas > > * mpt > > * pvscsi > > * virtio > > * virtio-blk > > > > As specified in the commit message. > > > > Also Gerd it seems that my lchs patches were not committed in the > > latest submitted version (v4)!!! > > The ABI of the fw config key is completely broken. > > > > > > On Wed, Nov 13, 2019 at 4:02 PM Gerd Hoffmann wrote: > > > > > > On Wed, Nov 13, 2019 at 10:35:03AM +0100, Laszlo Ersek wrote: > > > > On 11/13/19 10:18, Gerd Hoffmann wrote: > > > > > Specifically port->driver.lchs needs clearing, otherwise seabios will > > > > > > > > s/driver/drive/ > > > > > > > Reviewed-by: Laszlo Ersek > > > > > > Typo fixed & pushed. > > > > > > thanks, > > > Gerd > > > ___ > > > SeaBIOS mailing list -- seab...@seabios.org > > > To unsubscribe send an email to seabios-le...@seabios.org
Re: [SeaBIOS] Re: [PATCH] ahci: zero-initialize port struct
We should make sure drive.lchs is zeroed out for the following devices: git grep "drive_s drive" src/hw/ata.h:struct drive_s drive; src/hw/esp-scsi.c:struct drive_s drive; src/hw/lsi-scsi.c:struct drive_s drive; src/hw/megasas.c:struct drive_s drive; src/hw/mpt-scsi.c:struct drive_s drive; src/hw/nvme-int.h:struct drive_s drive; src/hw/pvscsi.c:struct drive_s drive; src/hw/sdcard.c:struct drive_s drive; src/hw/usb-msc.c:struct drive_s drive; src/hw/usb-uas.c:struct drive_s drive; src/hw/virtio-blk.c:struct drive_s drive; src/hw/virtio-scsi.c:struct drive_s drive; Sam On Wed, Nov 13, 2019 at 5:03 PM Sam Eiderman wrote: > > Hi, > > Does this fix a bug that actually happened? > > I just noticed that in my lchs patches I assumed that lchs struct is > zeroed out in all devices (not only ahci): > > 9caa19be0e53 (geometry: Apply LCHS values for boot devices) > > Seems like this is not the case but why only ahci is affected? > > The list of devices is at least: > > * ata > * ahci > * scsi > * esp > * lsi > * megasas > * mpt > * pvscsi > * virtio > * virtio-blk > > As specified in the commit message. > > Also Gerd it seems that my lchs patches were not committed in the > latest submitted version (v4)!!! > The ABI of the fw config key is completely broken. > > > On Wed, Nov 13, 2019 at 4:02 PM Gerd Hoffmann wrote: > > > > On Wed, Nov 13, 2019 at 10:35:03AM +0100, Laszlo Ersek wrote: > > > On 11/13/19 10:18, Gerd Hoffmann wrote: > > > > Specifically port->driver.lchs needs clearing, otherwise seabios will > > > > > > s/driver/drive/ > > > > > Reviewed-by: Laszlo Ersek > > > > Typo fixed & pushed. > > > > thanks, > > Gerd > > ___ > > SeaBIOS mailing list -- seab...@seabios.org > > To unsubscribe send an email to seabios-le...@seabios.org
Re: [SeaBIOS] Re: [PATCH] ahci: zero-initialize port struct
Hi, Does this fix a bug that actually happened? I just noticed that in my lchs patches I assumed that lchs struct is zeroed out in all devices (not only ahci): 9caa19be0e53 (geometry: Apply LCHS values for boot devices) Seems like this is not the case but why only ahci is affected? The list of devices is at least: * ata * ahci * scsi * esp * lsi * megasas * mpt * pvscsi * virtio * virtio-blk As specified in the commit message. Also Gerd it seems that my lchs patches were not committed in the latest submitted version (v4)!!! The ABI of the fw config key is completely broken. On Wed, Nov 13, 2019 at 4:02 PM Gerd Hoffmann wrote: > > On Wed, Nov 13, 2019 at 10:35:03AM +0100, Laszlo Ersek wrote: > > On 11/13/19 10:18, Gerd Hoffmann wrote: > > > Specifically port->driver.lchs needs clearing, otherwise seabios will > > > > s/driver/drive/ > > > Reviewed-by: Laszlo Ersek > > Typo fixed & pushed. > > thanks, > Gerd > ___ > SeaBIOS mailing list -- seab...@seabios.org > To unsubscribe send an email to seabios-le...@seabios.org
[PATCH v8 8/8] hd-geo-test: Add tests for lchs override
From: Sam Eiderman
Add QTest tests to check the logical geometry override option.
The tests in hd-geo-test are out of date - they only test IDE and do not
test interesting MBRs.
Creating qcow2 disks with specific size and MBR layout is currently
unused - we only use a default empty MBR.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
Signed-off-by: Sam Eiderman
---
tests/Makefile.include | 2 +-
tests/hd-geo-test.c| 551 +
2 files changed, 552 insertions(+), 1 deletion(-)
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3543451ed3..6941ae7c77 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -780,7 +780,7 @@ tests/ide-test$(EXESUF): tests/ide-test.o $(libqos-pc-obj-y)
tests/ahci-test$(EXESUF): tests/ahci-test.o $(libqos-pc-obj-y)
qemu-img$(EXESUF)
tests/ipmi-kcs-test$(EXESUF): tests/ipmi-kcs-test.o
tests/ipmi-bt-test$(EXESUF): tests/ipmi-bt-test.o
-tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o
+tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o $(libqos-obj-y)
tests/boot-order-test$(EXESUF): tests/boot-order-test.o $(libqos-obj-y)
tests/boot-serial-test$(EXESUF): tests/boot-serial-test.o $(libqos-obj-y)
tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
index 62eb624726..7e86c5416c 100644
--- a/tests/hd-geo-test.c
+++ b/tests/hd-geo-test.c
@@ -17,7 +17,12 @@
#include "qemu/osdep.h"
#include "qemu-common.h"
+#include "qemu/bswap.h"
+#include "qapi/qmp/qlist.h"
#include "libqtest.h"
+#include "libqos/fw_cfg.h"
+#include "libqos/libqos.h"
+#include "standard-headers/linux/qemu_fw_cfg.h"
#define ARGV_SIZE 256
@@ -388,6 +393,537 @@ static void test_ide_drive_cd_0(void)
qtest_quit(qts);
}
+typedef struct {
+bool active;
+uint32_t head;
+uint32_t sector;
+uint32_t cyl;
+uint32_t end_head;
+uint32_t end_sector;
+uint32_t end_cyl;
+uint32_t start_sect;
+uint32_t nr_sects;
+} MBRpartitions[4];
+
+static MBRpartitions empty_mbr = { {false, 0, 0, 0, 0, 0, 0, 0, 0},
+ {false, 0, 0, 0, 0, 0, 0, 0, 0},
+ {false, 0, 0, 0, 0, 0, 0, 0, 0},
+ {false, 0, 0, 0, 0, 0, 0, 0, 0} };
+
+static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
+{
+const char *template = "/tmp/qtest.XX";
+char *raw_path = strdup(template);
+char *qcow2_path = strdup(template);
+char cmd[100 + 2 * PATH_MAX];
+uint8_t buf[512];
+int i, ret, fd, offset;
+uint64_t qcow2_size = sectors * 512;
+uint8_t status, parttype, head, sector, cyl;
+char *qemu_img_path;
+char *qemu_img_abs_path;
+
+offset = 0xbe;
+
+for (i = 0; i < 4; i++) {
+status = mbr[i].active ? 0x80 : 0x00;
+g_assert(mbr[i].head < 256);
+g_assert(mbr[i].sector < 64);
+g_assert(mbr[i].cyl < 1024);
+head = mbr[i].head;
+sector = mbr[i].sector + ((mbr[i].cyl & 0x300) >> 2);
+cyl = mbr[i].cyl & 0xff;
+
+buf[offset + 0x0] = status;
+buf[offset + 0x1] = head;
+buf[offset + 0x2] = sector;
+buf[offset + 0x3] = cyl;
+
+parttype = 0;
+g_assert(mbr[i].end_head < 256);
+g_assert(mbr[i].end_sector < 64);
+g_assert(mbr[i].end_cyl < 1024);
+head = mbr[i].end_head;
+sector = mbr[i].end_sector + ((mbr[i].end_cyl & 0x300) >> 2);
+cyl = mbr[i].end_cyl & 0xff;
+
+buf[offset + 0x4] = parttype;
+buf[offset + 0x5] = head;
+buf[offset + 0x6] = sector;
+buf[offset + 0x7] = cyl;
+
+(*(uint32_t *)&buf[offset + 0x8]) = cpu_to_le32(mbr[i].start_sect);
+(*(uint32_t *)&buf[offset + 0xc]) = cpu_to_le32(mbr[i].nr_sects);
+
+offset += 0x10;
+}
+
+fd = mkstemp(raw_path);
+g_assert(fd);
+close(fd);
+
+fd = open(raw_path, O_WRONLY);
+g_assert(fd >= 0);
+ret = write(fd, buf, sizeof(buf));
+g_assert(ret == sizeof(buf));
+close(fd);
+
+fd = mkstemp(qcow2_path);
+g_assert(fd);
+close(fd);
+
+qemu_img_path = getenv("QTEST_QEMU_IMG");
+g_assert(qemu_img_path);
+qemu_img_abs_path = realpath(qemu_img_path, NULL);
+g_assert(qemu_img_abs_path);
+
+ret = snprintf(cmd, sizeof(cmd),
+ "%s convert -f raw -O qcow2 %s %s > /dev/null",
+ qemu_img_abs_path,
+ raw_path, qcow2_path);
+g_assert((0 < ret) && (ret <= sizeof(cmd)));
+ret = system(cmd);
+g_assert(ret == 0);
+
+ret = snprintf(cmd, sizeof(cmd),
+ "%s resize %s %" PRIu64 " > /dev/null",
+ qemu_img_abs_path,
[PATCH v8 7/8] bootdevice: FW_CFG interface for LCHS values
From: Sam Eiderman
Using fw_cfg, supply logical CHS values directly from QEMU to the BIOS.
Non-standard logical geometries break under QEMU.
A virtual disk which contains an operating system which depends on
logical geometries (consistent values being reported from BIOS INT13
AH=08) will most likely break under QEMU/SeaBIOS if it has non-standard
logical geometries - for example 56 SPT (sectors per track).
No matter what QEMU will report - SeaBIOS, for large enough disks - will
use LBA translation, which will report 63 SPT instead.
In addition we cannot force SeaBIOS to rely on physical geometries at
all. A virtio-blk-pci virtual disk with 255 phyiscal heads cannot
report more than 16 physical heads when moved to an IDE controller,
since the ATA spec allows a maximum of 16 heads - this is an artifact of
virtualization.
By supplying the logical geometries directly we are able to support such
"exotic" disks.
We serialize this information in a similar way to the "bootorder"
interface.
The new fw_cfg entry is "bios-geometry".
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Sam Eiderman
Signed-off-by: Sam Eiderman
---
bootdevice.c| 31 +++
hw/nvram/fw_cfg.c | 14 +++---
include/sysemu/sysemu.h | 1 +
3 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/bootdevice.c b/bootdevice.c
index 2cf6b37c57..03aaffcc8d 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -405,3 +405,34 @@ void del_boot_device_lchs(DeviceState *dev, const char
*suffix)
}
}
}
+
+char *get_boot_devices_lchs_list(size_t *size)
+{
+FWLCHSEntry *i;
+size_t total = 0;
+char *list = NULL;
+
+QTAILQ_FOREACH(i, &fw_lchs, link) {
+char *bootpath;
+char *chs_string;
+size_t len;
+
+bootpath = get_boot_device_path(i->dev, false, i->suffix);
+chs_string = g_strdup_printf("%s %" PRIu32 " %" PRIu32 " %" PRIu32,
+ bootpath, i->lcyls, i->lheads, i->lsecs);
+
+if (total) {
+list[total - 1] = '\n';
+}
+len = strlen(chs_string) + 1;
+list = g_realloc(list, total + len);
+memcpy(&list[total], chs_string, len);
+total += len;
+g_free(chs_string);
+g_free(bootpath);
+}
+
+*size = total;
+
+return list;
+}
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 7dc3ac378e..18aff658c0 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -920,13 +920,21 @@ void *fw_cfg_modify_file(FWCfgState *s, const char
*filename,
static void fw_cfg_machine_reset(void *opaque)
{
+MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
+FWCfgState *s = opaque;
void *ptr;
size_t len;
-FWCfgState *s = opaque;
-char *bootindex = get_boot_devices_list(&len);
+char *buf;
-ptr = fw_cfg_modify_file(s, "bootorder", (uint8_t *)bootindex, len);
+buf = get_boot_devices_list(&len);
+ptr = fw_cfg_modify_file(s, "bootorder", (uint8_t *)buf, len);
g_free(ptr);
+
+if (!mc->legacy_fw_cfg_order) {
+buf = get_boot_devices_lchs_list(&len);
+ptr = fw_cfg_modify_file(s, "bios-geometry", (uint8_t *)buf, len);
+g_free(ptr);
+}
}
static void fw_cfg_machine_ready(struct Notifier *n, void *data)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 5bc5c79cbc..80c57fdc4e 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -106,6 +106,7 @@ void validate_bootdevices(const char *devices, Error
**errp);
void add_boot_device_lchs(DeviceState *dev, const char *suffix,
uint32_t lcyls, uint32_t lheads, uint32_t lsecs);
void del_boot_device_lchs(DeviceState *dev, const char *suffix);
+char *get_boot_devices_lchs_list(size_t *size);
/* handler to set the boot_device order for a specific type of MachineClass */
typedef void QEMUBootSetHandler(void *opaque, const char *boot_order,
--
2.23.0.700.g56cf767bdb-goog
[PATCH v8 5/8] bootdevice: Gather LCHS from all relevant devices
From: Sam Eiderman
Relevant devices are:
* ide-hd (and ide-cd, ide-drive)
* scsi-hd (and scsi-cd, scsi-disk, scsi-block)
* virtio-blk-pci
We do not call del_boot_device_lchs() for ide-* since we don't need to -
IDE block devices do not support unplugging.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
Signed-off-by: Sam Eiderman
---
hw/block/virtio-blk.c | 6 ++
hw/ide/qdev.c | 5 +
hw/scsi/scsi-disk.c | 12
3 files changed, 23 insertions(+)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index ed2ddebd2b..c56e905f80 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1200,6 +1200,11 @@ static void virtio_blk_device_realize(DeviceState *dev,
Error **errp)
blk_set_guest_block_size(s->blk, s->conf.conf.logical_block_size);
blk_iostatus_enable(s->blk);
+
+add_boot_device_lchs(dev, "/disk@0,0",
+ conf->conf.lcyls,
+ conf->conf.lheads,
+ conf->conf.lsecs);
}
static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp)
@@ -1207,6 +1212,7 @@ static void virtio_blk_device_unrealize(DeviceState *dev,
Error **errp)
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
VirtIOBlock *s = VIRTIO_BLK(dev);
+del_boot_device_lchs(dev, "/disk@0,0");
virtio_blk_data_plane_destroy(s->dataplane);
s->dataplane = NULL;
qemu_del_vm_change_state_handler(s->change);
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 6dd219944f..2ffd387a73 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -220,6 +220,11 @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind
kind, Error **errp)
add_boot_device_path(dev->conf.bootindex, &dev->qdev,
dev->unit ? "/disk@1" : "/disk@0");
+
+add_boot_device_lchs(&dev->qdev, dev->unit ? "/disk@1" : "/disk@0",
+ dev->conf.lcyls,
+ dev->conf.lheads,
+ dev->conf.lsecs);
}
static void ide_dev_get_bootindex(Object *obj, Visitor *v, const char *name,
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 68b1675fd9..07fb5ebdf1 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -35,6 +35,7 @@
#include "hw/block/block.h"
#include "hw/qdev-properties.h"
#include "sysemu/dma.h"
+#include "sysemu/sysemu.h"
#include "qemu/cutils.h"
#include "trace.h"
@@ -2414,6 +2415,16 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
blk_set_guest_block_size(s->qdev.conf.blk, s->qdev.blocksize);
blk_iostatus_enable(s->qdev.conf.blk);
+
+add_boot_device_lchs(&dev->qdev, NULL,
+ dev->conf.lcyls,
+ dev->conf.lheads,
+ dev->conf.lsecs);
+}
+
+static void scsi_unrealize(SCSIDevice *dev, Error **errp)
+{
+del_boot_device_lchs(&dev->qdev, NULL);
}
static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
@@ -3018,6 +3029,7 @@ static void scsi_hd_class_initfn(ObjectClass *klass, void
*data)
SCSIDeviceClass *sc = SCSI_DEVICE_CLASS(klass);
sc->realize = scsi_hd_realize;
+sc->unrealize= scsi_unrealize;
sc->alloc_req= scsi_new_request;
sc->unit_attention_reported = scsi_disk_unit_attention_reported;
dc->desc = "virtual SCSI disk";
--
2.23.0.700.g56cf767bdb-goog
[PATCH v8 6/8] bootdevice: Refactor get_boot_devices_list
From: Sam Eiderman
Move device name construction to a separate function.
We will reuse this function in the following commit to pass logical CHS
parameters through fw_cfg much like we currently pass bootindex.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Sam Eiderman
Signed-off-by: Sam Eiderman
---
bootdevice.c | 61 +---
1 file changed, 34 insertions(+), 27 deletions(-)
diff --git a/bootdevice.c b/bootdevice.c
index bc5e1c2de4..2cf6b37c57 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -202,6 +202,39 @@ DeviceState *get_boot_device(uint32_t position)
return res;
}
+static char *get_boot_device_path(DeviceState *dev, bool ignore_suffixes,
+ const char *suffix)
+{
+char *devpath = NULL, *s = NULL, *d, *bootpath;
+
+if (dev) {
+devpath = qdev_get_fw_dev_path(dev);
+assert(devpath);
+}
+
+if (!ignore_suffixes) {
+if (dev) {
+d = qdev_get_own_fw_dev_path_from_handler(dev->parent_bus, dev);
+if (d) {
+assert(!suffix);
+s = d;
+} else {
+s = g_strdup(suffix);
+}
+} else {
+s = g_strdup(suffix);
+}
+}
+
+bootpath = g_strdup_printf("%s%s",
+ devpath ? devpath : "",
+ s ? s : "");
+g_free(devpath);
+g_free(s);
+
+return bootpath;
+}
+
/*
* This function returns null terminated string that consist of new line
* separated device paths.
@@ -218,36 +251,10 @@ char *get_boot_devices_list(size_t *size)
bool ignore_suffixes = mc->ignore_boot_device_suffixes;
QTAILQ_FOREACH(i, &fw_boot_order, link) {
-char *devpath = NULL, *suffix = NULL;
char *bootpath;
-char *d;
size_t len;
-if (i->dev) {
-devpath = qdev_get_fw_dev_path(i->dev);
-assert(devpath);
-}
-
-if (!ignore_suffixes) {
-if (i->dev) {
-d = qdev_get_own_fw_dev_path_from_handler(i->dev->parent_bus,
- i->dev);
-if (d) {
-assert(!i->suffix);
-suffix = d;
-} else {
-suffix = g_strdup(i->suffix);
-}
-} else {
-suffix = g_strdup(i->suffix);
-}
-}
-
-bootpath = g_strdup_printf("%s%s",
- devpath ? devpath : "",
- suffix ? suffix : "");
-g_free(devpath);
-g_free(suffix);
+bootpath = get_boot_device_path(i->dev, ignore_suffixes, i->suffix);
if (total) {
list[total-1] = '\n';
--
2.23.0.700.g56cf767bdb-goog
[PATCH v8 4/8] scsi: Propagate unrealize() callback to scsi-hd
From: Sam Eiderman
We will need to add LCHS removal logic to scsi-hd's unrealize() in the
next commit.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Sam Eiderman
Signed-off-by: Sam Eiderman
---
hw/scsi/scsi-bus.c | 16
include/hw/scsi/scsi.h | 1 +
2 files changed, 17 insertions(+)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index bccb7cc4c6..359d50d6d0 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -59,6 +59,14 @@ static void scsi_device_realize(SCSIDevice *s, Error **errp)
}
}
+static void scsi_device_unrealize(SCSIDevice *s, Error **errp)
+{
+SCSIDeviceClass *sc = SCSI_DEVICE_GET_CLASS(s);
+if (sc->unrealize) {
+sc->unrealize(s, errp);
+}
+}
+
int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
void *hba_private)
{
@@ -217,12 +225,20 @@ static void scsi_qdev_realize(DeviceState *qdev, Error
**errp)
static void scsi_qdev_unrealize(DeviceState *qdev, Error **errp)
{
SCSIDevice *dev = SCSI_DEVICE(qdev);
+Error *local_err = NULL;
if (dev->vmsentry) {
qemu_del_vm_change_state_handler(dev->vmsentry);
}
scsi_device_purge_requests(dev, SENSE_CODE(NO_SENSE));
+
+scsi_device_unrealize(dev, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
blockdev_mark_auto_del(dev->conf.blk);
}
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index d77a92361b..332ef602f4 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -59,6 +59,7 @@ struct SCSIRequest {
typedef struct SCSIDeviceClass {
DeviceClass parent_class;
void (*realize)(SCSIDevice *dev, Error **errp);
+void (*unrealize)(SCSIDevice *dev, Error **errp);
int (*parse_cdb)(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
void *hba_private);
SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun,
--
2.23.0.700.g56cf767bdb-goog
[PATCH v8 1/8] block: Refactor macros - fix tabbing
From: Sam Eiderman
Fixing tabbing in block related macros.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Sam Eiderman
Signed-off-by: Sam Eiderman
---
hw/ide/qdev.c| 2 +-
include/hw/block/block.h | 16
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 6fba6b62b8..6dd219944f 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -290,7 +290,7 @@ static void ide_drive_realize(IDEDevice *dev, Error **errp)
DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf),\
DEFINE_BLOCK_ERROR_PROPERTIES(IDEDrive, dev.conf), \
DEFINE_PROP_STRING("ver", IDEDrive, dev.version), \
-DEFINE_PROP_UINT64("wwn", IDEDrive, dev.wwn, 0),\
+DEFINE_PROP_UINT64("wwn", IDEDrive, dev.wwn, 0), \
DEFINE_PROP_STRING("serial", IDEDrive, dev.serial),\
DEFINE_PROP_STRING("model", IDEDrive, dev.model)
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index 607539057a..fd55a30bca 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -50,21 +50,21 @@ static inline unsigned int get_physical_block_exp(BlockConf
*conf)
_conf.logical_block_size),\
DEFINE_PROP_BLOCKSIZE("physical_block_size", _state,\
_conf.physical_block_size), \
-DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0), \
+DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),\
DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0),\
-DEFINE_PROP_UINT32("discard_granularity", _state, \
- _conf.discard_granularity, -1), \
-DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, \
-ON_OFF_AUTO_AUTO), \
+DEFINE_PROP_UINT32("discard_granularity", _state, \
+ _conf.discard_granularity, -1), \
+DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, \
+ON_OFF_AUTO_AUTO), \
DEFINE_PROP_BOOL("share-rw", _state, _conf.share_rw, false)
#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \
DEFINE_PROP_DRIVE("drive", _state, _conf.blk), \
DEFINE_BLOCK_PROPERTIES_BASE(_state, _conf)
-#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \
-DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \
-DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \
+#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \
+DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \
+DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0),\
DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0)
#define DEFINE_BLOCK_ERROR_PROPERTIES(_state, _conf)\
--
2.23.0.700.g56cf767bdb-goog
[PATCH v8 3/8] bootdevice: Add interface to gather LCHS
From: Sam Eiderman
Add an interface to provide direct logical CHS values for boot devices.
We will use this interface in the next commits.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
Signed-off-by: Sam Eiderman
---
bootdevice.c| 55 +
include/sysemu/sysemu.h | 3 +++
2 files changed, 58 insertions(+)
diff --git a/bootdevice.c b/bootdevice.c
index 1d225202f9..bc5e1c2de4 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -343,3 +343,58 @@ void device_add_bootindex_property(Object *obj, int32_t
*bootindex,
/* initialize devices' bootindex property to -1 */
object_property_set_int(obj, -1, name, NULL);
}
+
+typedef struct FWLCHSEntry FWLCHSEntry;
+
+struct FWLCHSEntry {
+QTAILQ_ENTRY(FWLCHSEntry) link;
+DeviceState *dev;
+char *suffix;
+uint32_t lcyls;
+uint32_t lheads;
+uint32_t lsecs;
+};
+
+static QTAILQ_HEAD(, FWLCHSEntry) fw_lchs =
+QTAILQ_HEAD_INITIALIZER(fw_lchs);
+
+void add_boot_device_lchs(DeviceState *dev, const char *suffix,
+ uint32_t lcyls, uint32_t lheads, uint32_t lsecs)
+{
+FWLCHSEntry *node;
+
+if (!lcyls && !lheads && !lsecs) {
+return;
+}
+
+assert(dev != NULL || suffix != NULL);
+
+node = g_malloc0(sizeof(FWLCHSEntry));
+node->suffix = g_strdup(suffix);
+node->dev = dev;
+node->lcyls = lcyls;
+node->lheads = lheads;
+node->lsecs = lsecs;
+
+QTAILQ_INSERT_TAIL(&fw_lchs, node, link);
+}
+
+void del_boot_device_lchs(DeviceState *dev, const char *suffix)
+{
+FWLCHSEntry *i;
+
+if (dev == NULL) {
+return;
+}
+
+QTAILQ_FOREACH(i, &fw_lchs, link) {
+if ((!suffix || !g_strcmp0(i->suffix, suffix)) &&
+ i->dev == dev) {
+QTAILQ_REMOVE(&fw_lchs, i, link);
+g_free(i->suffix);
+g_free(i);
+
+break;
+}
+}
+}
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 44f18eb739..5bc5c79cbc 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -103,6 +103,9 @@ void device_add_bootindex_property(Object *obj, int32_t
*bootindex,
DeviceState *dev, Error **errp);
void restore_boot_order(void *opaque);
void validate_bootdevices(const char *devices, Error **errp);
+void add_boot_device_lchs(DeviceState *dev, const char *suffix,
+ uint32_t lcyls, uint32_t lheads, uint32_t lsecs);
+void del_boot_device_lchs(DeviceState *dev, const char *suffix);
/* handler to set the boot_device order for a specific type of MachineClass */
typedef void QEMUBootSetHandler(void *opaque, const char *boot_order,
--
2.23.0.700.g56cf767bdb-goog
[PATCH v8 1/1] hd-geo-test: Add tests for lchs override
From: Sam Eiderman
Add QTest tests to check the logical geometry override option.
The tests in hd-geo-test are out of date - they only test IDE and do not
test interesting MBRs.
Creating qcow2 disks with specific size and MBR layout is currently
unused - we only use a default empty MBR.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
Signed-off-by: Sam Eiderman
---
tests/Makefile.include | 2 +-
tests/hd-geo-test.c| 551 +
2 files changed, 552 insertions(+), 1 deletion(-)
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3543451ed3..6941ae7c77 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -780,7 +780,7 @@ tests/ide-test$(EXESUF): tests/ide-test.o $(libqos-pc-obj-y)
tests/ahci-test$(EXESUF): tests/ahci-test.o $(libqos-pc-obj-y)
qemu-img$(EXESUF)
tests/ipmi-kcs-test$(EXESUF): tests/ipmi-kcs-test.o
tests/ipmi-bt-test$(EXESUF): tests/ipmi-bt-test.o
-tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o
+tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o $(libqos-obj-y)
tests/boot-order-test$(EXESUF): tests/boot-order-test.o $(libqos-obj-y)
tests/boot-serial-test$(EXESUF): tests/boot-serial-test.o $(libqos-obj-y)
tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
index 62eb624726..7e86c5416c 100644
--- a/tests/hd-geo-test.c
+++ b/tests/hd-geo-test.c
@@ -17,7 +17,12 @@
#include "qemu/osdep.h"
#include "qemu-common.h"
+#include "qemu/bswap.h"
+#include "qapi/qmp/qlist.h"
#include "libqtest.h"
+#include "libqos/fw_cfg.h"
+#include "libqos/libqos.h"
+#include "standard-headers/linux/qemu_fw_cfg.h"
#define ARGV_SIZE 256
@@ -388,6 +393,537 @@ static void test_ide_drive_cd_0(void)
qtest_quit(qts);
}
+typedef struct {
+bool active;
+uint32_t head;
+uint32_t sector;
+uint32_t cyl;
+uint32_t end_head;
+uint32_t end_sector;
+uint32_t end_cyl;
+uint32_t start_sect;
+uint32_t nr_sects;
+} MBRpartitions[4];
+
+static MBRpartitions empty_mbr = { {false, 0, 0, 0, 0, 0, 0, 0, 0},
+ {false, 0, 0, 0, 0, 0, 0, 0, 0},
+ {false, 0, 0, 0, 0, 0, 0, 0, 0},
+ {false, 0, 0, 0, 0, 0, 0, 0, 0} };
+
+static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
+{
+const char *template = "/tmp/qtest.XX";
+char *raw_path = strdup(template);
+char *qcow2_path = strdup(template);
+char cmd[100 + 2 * PATH_MAX];
+uint8_t buf[512];
+int i, ret, fd, offset;
+uint64_t qcow2_size = sectors * 512;
+uint8_t status, parttype, head, sector, cyl;
+char *qemu_img_path;
+char *qemu_img_abs_path;
+
+offset = 0xbe;
+
+for (i = 0; i < 4; i++) {
+status = mbr[i].active ? 0x80 : 0x00;
+g_assert(mbr[i].head < 256);
+g_assert(mbr[i].sector < 64);
+g_assert(mbr[i].cyl < 1024);
+head = mbr[i].head;
+sector = mbr[i].sector + ((mbr[i].cyl & 0x300) >> 2);
+cyl = mbr[i].cyl & 0xff;
+
+buf[offset + 0x0] = status;
+buf[offset + 0x1] = head;
+buf[offset + 0x2] = sector;
+buf[offset + 0x3] = cyl;
+
+parttype = 0;
+g_assert(mbr[i].end_head < 256);
+g_assert(mbr[i].end_sector < 64);
+g_assert(mbr[i].end_cyl < 1024);
+head = mbr[i].end_head;
+sector = mbr[i].end_sector + ((mbr[i].end_cyl & 0x300) >> 2);
+cyl = mbr[i].end_cyl & 0xff;
+
+buf[offset + 0x4] = parttype;
+buf[offset + 0x5] = head;
+buf[offset + 0x6] = sector;
+buf[offset + 0x7] = cyl;
+
+(*(uint32_t *)&buf[offset + 0x8]) = cpu_to_le32(mbr[i].start_sect);
+(*(uint32_t *)&buf[offset + 0xc]) = cpu_to_le32(mbr[i].nr_sects);
+
+offset += 0x10;
+}
+
+fd = mkstemp(raw_path);
+g_assert(fd);
+close(fd);
+
+fd = open(raw_path, O_WRONLY);
+g_assert(fd >= 0);
+ret = write(fd, buf, sizeof(buf));
+g_assert(ret == sizeof(buf));
+close(fd);
+
+fd = mkstemp(qcow2_path);
+g_assert(fd);
+close(fd);
+
+qemu_img_path = getenv("QTEST_QEMU_IMG");
+g_assert(qemu_img_path);
+qemu_img_abs_path = realpath(qemu_img_path, NULL);
+g_assert(qemu_img_abs_path);
+
+ret = snprintf(cmd, sizeof(cmd),
+ "%s convert -f raw -O qcow2 %s %s > /dev/null",
+ qemu_img_abs_path,
+ raw_path, qcow2_path);
+g_assert((0 < ret) && (ret <= sizeof(cmd)));
+ret = system(cmd);
+g_assert(ret == 0);
+
+ret = snprintf(cmd, sizeof(cmd),
+ "%s resize %s %" PRIu64 " > /dev/null",
+ qemu_img_abs_path,
[PATCH v8 2/8] block: Support providing LCHS from user
From: Sam Eiderman
Add logical geometry variables to BlockConf.
A user can now supply "lcyls", "lheads" & "lsecs" for any HD device
that supports CHS ("cyls", "heads", "secs").
These devices include:
* ide-hd
* scsi-hd
* virtio-blk-pci
In future commits we will use the provided LCHS and pass it to the BIOS
through fw_cfg to be supplied using INT13 routines.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
Signed-off-by: Sam Eiderman
---
include/hw/block/block.h | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index fd55a30bca..d7246f3862 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -26,6 +26,7 @@ typedef struct BlockConf {
uint32_t discard_granularity;
/* geometry, not all devices use this */
uint32_t cyls, heads, secs;
+uint32_t lcyls, lheads, lsecs;
OnOffAuto wce;
bool share_rw;
BlockdevOnError rerror;
@@ -65,7 +66,10 @@ static inline unsigned int get_physical_block_exp(BlockConf
*conf)
#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \
DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \
DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0),\
-DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0)
+DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0), \
+DEFINE_PROP_UINT32("lcyls", _state, _conf.lcyls, 0),\
+DEFINE_PROP_UINT32("lheads", _state, _conf.lheads, 0), \
+DEFINE_PROP_UINT32("lsecs", _state, _conf.lsecs, 0)
#define DEFINE_BLOCK_ERROR_PROPERTIES(_state, _conf)\
DEFINE_PROP_BLOCKDEV_ON_ERROR("rerror", _state, _conf.rerror, \
--
2.23.0.700.g56cf767bdb-goog
[PATCH v8 0/8] Add Qemu to SeaBIOS LCHS interface
v1: Non-standard logical geometries break under QEMU. A virtual disk which contains an operating system which depends on logical geometries (consistent values being reported from BIOS INT13 AH=08) will most likely break under QEMU/SeaBIOS if it has non-standard logical geometries - for example 56 SPT (sectors per track). No matter what QEMU will guess - SeaBIOS, for large enough disks - will use LBA translation, which will report 63 SPT instead. In addition we can not enforce SeaBIOS to rely on phyiscal geometries at all. A virtio-blk-pci virtual disk with 255 phyiscal heads can not report more than 16 physical heads when moved to an IDE controller, the ATA spec allows a maximum of 16 heads - this is an artifact of virtualization. By supplying the logical geometies directly we are able to support such "exotic" disks. We will use fw_cfg to do just that. v2: Fix missing parenthesis check in "hd-geo-test: Add tests for lchs override" v3: * Rename fw_cfg key to "bios-geometry". * Remove "extendible" interface. * Add cpu_to_le32 fix as Laszlo suggested or big endian hosts * Fix last qtest commit - automatic docker tester for some reason does not have qemu-img set v4: * Change fw_cfg interface from mixed textual/binary to textual only v5: * Fix line > 80 chars in tests/hd-geo-test.c v6: * Small fixes for issues pointed by Max * (&conf->conf)->lcyls to conf->conf.lcyls and so on * Remove scsi_unrealize from everything other than scsi-hd * Add proper include to sysemu.h * scsi_device_unrealize() after scsi_device_purge_requests() v7: * Adapted last commit (tests) to changes in qtest v8: * Fixed BE issue with tests by using qfw_cfg_get_file() instead of read_fw_cfg_file(), thanks Laszlo. * Removed incorrect comment in 7/8. Sam Eiderman (8): block: Refactor macros - fix tabbing block: Support providing LCHS from user bootdevice: Add interface to gather LCHS scsi: Propagate unrealize() callback to scsi-hd bootdevice: Gather LCHS from all relevant devices bootdevice: Refactor get_boot_devices_list bootdevice: FW_CFG interface for LCHS values hd-geo-test: Add tests for lchs override bootdevice.c | 147 +-- hw/block/virtio-blk.c| 6 + hw/ide/qdev.c| 7 +- hw/nvram/fw_cfg.c| 14 +- hw/scsi/scsi-bus.c | 16 ++ hw/scsi/scsi-disk.c | 12 + include/hw/block/block.h | 22 +- include/hw/scsi/scsi.h | 1 + include/sysemu/sysemu.h | 4 + tests/Makefile.include | 2 +- tests/hd-geo-test.c | 551 +++ 11 files changed, 741 insertions(+), 41 deletions(-) -- 2.23.0.700.g56cf767bdb-goog
Re: [SeaBIOS] Re: [PATCH v7 7/8] bootdevice: FW_CFG interface for LCHS values
Sure! Philippe withdrew his R-b on 7/8, as I explained 7/8 is fine (only need to remove a bad comment) the problem was in the tests 8/8 - should I include the original R/b? I guess all other 1-6 are fine to add R/b... On Wed, Oct 16, 2019 at 6:07 PM John Snow wrote: > > > > On 10/16/19 10:55 AM, Sam Eiderman wrote: > > Thanks for the detailed comment Laszlo, > > > > Indeed my e-mail has changed and I only received replies to the > > commits where I added this new mail in the S-o-b section, should of > > added in all of them. > > > > So as you said it, the problem was actually in using qfw_cfg_get_u32 > > which assumes the value is encoded LE and has an additional > > le32_to_cpu, should have used qfw_cfg_get directly like > > qfw_cfg_get_file does. > > > > Regarding qfw_cfg_get_file - I wrote this code when this function did > > not exist yet, I think it was added 6 months ago. In any case, I will > > use it instead. > > > > Thanks for this. > > > > I will resubmit this entire commit series: > > * I will only change code in the last commit (tests) > > * I will remove a comment which is now not true anymore > > * I will add my new email in S-o-b > > > > Sam > > > > Philippe gave me a verbal tut-tut for not including his review tags in > my last pull request; when you re-spin could you be so kind as to > include any that still apply? > > --js
Re: [SeaBIOS] Re: [PATCH v7 7/8] bootdevice: FW_CFG interface for LCHS values
Thanks for the detailed comment Laszlo,
Indeed my e-mail has changed and I only received replies to the
commits where I added this new mail in the S-o-b section, should of
added in all of them.
So as you said it, the problem was actually in using qfw_cfg_get_u32
which assumes the value is encoded LE and has an additional
le32_to_cpu, should have used qfw_cfg_get directly like
qfw_cfg_get_file does.
Regarding qfw_cfg_get_file - I wrote this code when this function did
not exist yet, I think it was added 6 months ago. In any case, I will
use it instead.
Thanks for this.
I will resubmit this entire commit series:
* I will only change code in the last commit (tests)
* I will remove a comment which is now not true anymore
* I will add my new email in S-o-b
Sam
On Wed, Oct 16, 2019 at 3:29 PM Philippe Mathieu-Daudé
wrote:
>
> On 10/16/19 2:14 PM, Laszlo Ersek wrote:
> > Hi Sam,
> >
> > On 10/16/19 13:02, Sam Eiderman wrote:
> >> Gentle Ping,
> >>
> >> Philippe, John?
> >>
> >> Just wondering if the series is okay, as Gerd pointed out this series
> >> is a blocker for the corresponding changes in SeaBIOS for v 1.13
> >
> > The QEMU series is still not merged, due to a bug in the last patch
> > (namely, the test case, "hd-geo-test: Add tests for lchs override").
> >
> > To my knowledge, SeaBIOS prefers to merge patches with the underlying
> > QEMU patches merged first, so you'll likely have to fix that QEMU issue
> > first.
> >
> > I explained the bug in the QEMU test case here:
> >
> >
> > 6b00dc74-7267-8ce8-3271-5db269edb1b7@redhat.com">http://mid.mail-archive.com/6b00dc74-7267-8ce8-3271-5db269edb1b7@redhat.com
> >
> > 700cd594-1446-e478-fb03-d2e6b862dc6c@redhat.com">http://mid.mail-archive.com/700cd594-1446-e478-fb03-d2e6b862dc6c@redhat.com
>
> Yes, I was expecting a respin with find_fw_cfg_file() fixed per Laszlo
> detailed review.
>
> > (Alternative links to the same:
> >
> >https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg01790.html
> >https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg01793.html
> > )
> >
> > I've never received feedback to those messages, and I think you must
> > have missed them.
> >
> > FWIW, when I hit "Reply All" in that thread, you were on the "To:" list
> > with:
> >
> >Sam Eiderman
> >
> > but here you are present with
> >
> >Sam Eiderman
> >
> > In addition, when I posted those messages, I got the following
> > auto-response ("Undelivered Mail Returned to Sender"):
> >
> >> This is the mail system at host mx1.redhat.com.
> >>
> >> I'm sorry to have to inform you that your message could not
> >> be delivered to one or more recipients. It's attached below.
> >>
> >> For further assistance, please send mail to postmaster.
> >>
> >> If you do so, please include this problem report. You can
> >> delete your own text from the attached returned message.
> >>
> >> The mail system
> >>
> >> : host
> >> aserp2030.oracle.com[141.146.126.74] said:
> >> 550 5.1.1 Unknown recipient address. (in reply to RCPT TO command)
>
> That explains it :)
>
> >
> > I didn't know your new address, so I could only hope you'd find my
> > feedback on qemu-devel.
> >
> > Thanks
> > Laszlo
> >
Re: [SeaBIOS] [PATCH v7 7/8] bootdevice: FW_CFG interface for LCHS values
Gentle Ping, Philippe, John? Just wondering if the series is okay, as Gerd pointed out this series is a blocker for the corresponding changes in SeaBIOS for v 1.13 Sam On Tue, Oct 8, 2019 at 2:51 PM Sam Eiderman wrote: > > > > On Tue, Oct 8, 2019, 13:34 Philippe Mathieu-Daudé wrote: >> >> Hi Sam, >> >> On 9/29/19 12:13 PM, Sam Eiderman wrote: >> > Philippe, thanks for the fast review, >> >> Fast is not always the friend of careful. >> >> > >> > John, thanks for picking up this hot potato :-) >> > >> > Sam >> > >> > On Thu, Sep 26, 2019 at 10:16 PM Philippe Mathieu-Daudé >> > mailto:phi...@redhat.com>> wrote: >> > >> > On 9/26/19 9:09 PM, Philippe Mathieu-Daudé wrote: >> > > On 9/26/19 8:26 PM, John Snow wrote: >> > >> On 9/26/19 5:57 AM, Philippe Mathieu-Daudé wrote: >> > >>> Hi Sam, >> > >>> >> > >>> On 9/25/19 1:06 PM, Sam Eiderman wrote: >> > >>>> From: Sam Eiderman > > <mailto:shmuel.eider...@oracle.com>> >> > >>>> >> > >>>> Using fw_cfg, supply logical CHS values directly from QEMU to >> > the BIOS. >> > >>>> >> > >>>> Non-standard logical geometries break under QEMU. >> > >>>> >> > >>>> A virtual disk which contains an operating system which depends >> > on >> > >>>> logical geometries (consistent values being reported from BIOS >> > INT13 >> > >>>> AH=08) will most likely break under QEMU/SeaBIOS if it has >> > non-standard >> > >>>> logical geometries - for example 56 SPT (sectors per track). >> > >>>> No matter what QEMU will report - SeaBIOS, for large enough >> > disks - will >> > >>>> use LBA translation, which will report 63 SPT instead. >> > >>>> >> > >>>> In addition we cannot force SeaBIOS to rely on physical >> > geometries at >> > >>>> all. A virtio-blk-pci virtual disk with 255 phyiscal heads cannot >> > >>>> report more than 16 physical heads when moved to an IDE >> > controller, >> > >>>> since the ATA spec allows a maximum of 16 heads - this is an >> > artifact of >> > >>>> virtualization. >> > >>>> >> > >>>> By supplying the logical geometries directly we are able to >> > support such >> > >>>> "exotic" disks. >> > >>>> >> > >>>> We serialize this information in a similar way to the "bootorder" >> > >>>> interface. >> > >>>> The new fw_cfg entry is "bios-geometry". >> > >>>> >> > >>>> Reviewed-by: Karl Heubaum > > <mailto:karl.heub...@oracle.com>> >> > >>>> Reviewed-by: Arbel Moshe > > <mailto:arbel.mo...@oracle.com>> >> > >>>> Signed-off-by: Sam Eiderman > > <mailto:shmuel.eider...@oracle.com>> >> > >>>> --- >> > >>>> bootdevice.c| 32 >> > >>>> hw/nvram/fw_cfg.c | 14 +++--- >> > >>>> include/sysemu/sysemu.h | 1 + >> > >>>> 3 files changed, 44 insertions(+), 3 deletions(-) >> > >>>> >> > >>>> diff --git a/bootdevice.c b/bootdevice.c >> > >>>> index 2b12fb85a4..b034ad7bdc 100644 >> > >>>> --- a/bootdevice.c >> > >>>> +++ b/bootdevice.c >> > >>>> @@ -405,3 +405,35 @@ void del_boot_device_lchs(DeviceState >> > *dev, const char *suffix) >> > >>>> } >> > >>>> } >> > >>>> } >> > >>>> + >> > >>>> +/* Serialized as: (device name\0 + lchs struct) x devices */ >> >> I suppose the lchs struct is serialized in little-endian. > > > Nice catch, that's just a bad comment, should be removed. > There used to be a struct with 3 uint32_t values
Re: [SeaBIOS] [PATCH v7 7/8] bootdevice: FW_CFG interface for LCHS values
On Tue, Oct 8, 2019, 13:34 Philippe Mathieu-Daudé wrote:
> Hi Sam,
>
> On 9/29/19 12:13 PM, Sam Eiderman wrote:
> > Philippe, thanks for the fast review,
>
> Fast is not always the friend of careful.
>
> >
> > John, thanks for picking up this hot potato :-)
> >
> > Sam
> >
> > On Thu, Sep 26, 2019 at 10:16 PM Philippe Mathieu-Daudé
> > mailto:phi...@redhat.com>> wrote:
> >
> > On 9/26/19 9:09 PM, Philippe Mathieu-Daudé wrote:
> > > On 9/26/19 8:26 PM, John Snow wrote:
> > >> On 9/26/19 5:57 AM, Philippe Mathieu-Daudé wrote:
> > >>> Hi Sam,
> > >>>
> > >>> On 9/25/19 1:06 PM, Sam Eiderman wrote:
> > >>>> From: Sam Eiderman > <mailto:shmuel.eider...@oracle.com>>
> > >>>>
> > >>>> Using fw_cfg, supply logical CHS values directly from QEMU to
> > the BIOS.
> > >>>>
> > >>>> Non-standard logical geometries break under QEMU.
> > >>>>
> > >>>> A virtual disk which contains an operating system which
> depends on
> > >>>> logical geometries (consistent values being reported from BIOS
> > INT13
> > >>>> AH=08) will most likely break under QEMU/SeaBIOS if it has
> > non-standard
> > >>>> logical geometries - for example 56 SPT (sectors per track).
> > >>>> No matter what QEMU will report - SeaBIOS, for large enough
> > disks - will
> > >>>> use LBA translation, which will report 63 SPT instead.
> > >>>>
> > >>>> In addition we cannot force SeaBIOS to rely on physical
> > geometries at
> > >>>> all. A virtio-blk-pci virtual disk with 255 phyiscal heads
> cannot
> > >>>> report more than 16 physical heads when moved to an IDE
> > controller,
> > >>>> since the ATA spec allows a maximum of 16 heads - this is an
> > artifact of
> > >>>> virtualization.
> > >>>>
> > >>>> By supplying the logical geometries directly we are able to
> > support such
> > >>>> "exotic" disks.
> > >>>>
> > >>>> We serialize this information in a similar way to the
> "bootorder"
> > >>>> interface.
> > >>>> The new fw_cfg entry is "bios-geometry".
> > >>>>
> > >>>> Reviewed-by: Karl Heubaum > <mailto:karl.heub...@oracle.com>>
> > >>>> Reviewed-by: Arbel Moshe > <mailto:arbel.mo...@oracle.com>>
> > >>>> Signed-off-by: Sam Eiderman > <mailto:shmuel.eider...@oracle.com>>
> > >>>> ---
> > >>>> bootdevice.c| 32
> > >>>> hw/nvram/fw_cfg.c | 14 +++---
> > >>>> include/sysemu/sysemu.h | 1 +
> > >>>> 3 files changed, 44 insertions(+), 3 deletions(-)
> > >>>>
> > >>>> diff --git a/bootdevice.c b/bootdevice.c
> > >>>> index 2b12fb85a4..b034ad7bdc 100644
> > >>>> --- a/bootdevice.c
> > >>>> +++ b/bootdevice.c
> > >>>> @@ -405,3 +405,35 @@ void del_boot_device_lchs(DeviceState
> > *dev, const char *suffix)
> > >>>> }
> > >>>> }
> > >>>> }
> > >>>> +
> > >>>> +/* Serialized as: (device name\0 + lchs struct) x devices */
>
> I suppose the lchs struct is serialized in little-endian.
>
Nice catch, that's just a bad comment, should be removed.
There used to be a struct with 3 uint32_t values, Laszlo pointed out that
there is an endianess problem (this was fixed in v3) later Kevin suggested
to make it a textual interface and the struct was removed (in v4) but the
comment remained.
>
> > >>>> +char *get_boot_devices_lchs_list(size_t *size)
> > >>>> +{
> > >>>> +FWLCHSEntry *i;
> > >>>> +size_t total = 0;
> > >>>> +char *list = NULL;
> > >>>> +
> > >>>> +QTAILQ_FOREACH(i, &fw_lchs, link) {
> >
Re: [SeaBIOS] [PATCH v7 7/8] bootdevice: FW_CFG interface for LCHS values
Philippe, thanks for the fast review,
John, thanks for picking up this hot potato :-)
Sam
On Thu, Sep 26, 2019 at 10:16 PM Philippe Mathieu-Daudé
wrote:
> On 9/26/19 9:09 PM, Philippe Mathieu-Daudé wrote:
> > On 9/26/19 8:26 PM, John Snow wrote:
> >> On 9/26/19 5:57 AM, Philippe Mathieu-Daudé wrote:
> >>> Hi Sam,
> >>>
> >>> On 9/25/19 1:06 PM, Sam Eiderman wrote:
> >>>> From: Sam Eiderman
> >>>>
> >>>> Using fw_cfg, supply logical CHS values directly from QEMU to the
> BIOS.
> >>>>
> >>>> Non-standard logical geometries break under QEMU.
> >>>>
> >>>> A virtual disk which contains an operating system which depends on
> >>>> logical geometries (consistent values being reported from BIOS INT13
> >>>> AH=08) will most likely break under QEMU/SeaBIOS if it has
> non-standard
> >>>> logical geometries - for example 56 SPT (sectors per track).
> >>>> No matter what QEMU will report - SeaBIOS, for large enough disks -
> will
> >>>> use LBA translation, which will report 63 SPT instead.
> >>>>
> >>>> In addition we cannot force SeaBIOS to rely on physical geometries at
> >>>> all. A virtio-blk-pci virtual disk with 255 phyiscal heads cannot
> >>>> report more than 16 physical heads when moved to an IDE controller,
> >>>> since the ATA spec allows a maximum of 16 heads - this is an artifact
> of
> >>>> virtualization.
> >>>>
> >>>> By supplying the logical geometries directly we are able to support
> such
> >>>> "exotic" disks.
> >>>>
> >>>> We serialize this information in a similar way to the "bootorder"
> >>>> interface.
> >>>> The new fw_cfg entry is "bios-geometry".
> >>>>
> >>>> Reviewed-by: Karl Heubaum
> >>>> Reviewed-by: Arbel Moshe
> >>>> Signed-off-by: Sam Eiderman
> >>>> ---
> >>>> bootdevice.c| 32
> >>>> hw/nvram/fw_cfg.c | 14 +++---
> >>>> include/sysemu/sysemu.h | 1 +
> >>>> 3 files changed, 44 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/bootdevice.c b/bootdevice.c
> >>>> index 2b12fb85a4..b034ad7bdc 100644
> >>>> --- a/bootdevice.c
> >>>> +++ b/bootdevice.c
> >>>> @@ -405,3 +405,35 @@ void del_boot_device_lchs(DeviceState *dev,
> const char *suffix)
> >>>> }
> >>>> }
> >>>> }
> >>>> +
> >>>> +/* Serialized as: (device name\0 + lchs struct) x devices */
> >>>> +char *get_boot_devices_lchs_list(size_t *size)
> >>>> +{
> >>>> +FWLCHSEntry *i;
> >>>> +size_t total = 0;
> >>>> +char *list = NULL;
> >>>> +
> >>>> +QTAILQ_FOREACH(i, &fw_lchs, link) {
> >>>> +char *bootpath;
> >>>> +char *chs_string;
> >>>> +size_t len;
> >>>> +
> >>>> +bootpath = get_boot_device_path(i->dev, false, i->suffix);
> >>>> +chs_string = g_strdup_printf("%s %" PRIu32 " %" PRIu32 " %"
> PRIu32,
> >>>> + bootpath, i->lcyls, i->lheads,
> i->lsecs);
> >>>
> >>> Hmm maybe we can g_free(bootpath) directly here.
> >>>
> >>
> >> I think it's okay to do it at the bottom of the loop. No real benefit to
> >> being that eager to free resources in my mind. I expect setup at the top
> >> of a block and teardown at the bottom of a block.
> >>
> >> Trying to do too much in the middle gets messy in my opinion, not that
> >> it seems to matter here.
> >
> > No problem.
> >
> >>>> +
> >>>> +if (total) {
> >>>> +list[total - 1] = '\n';
> >>>> +}
> >>>> +len = strlen(chs_string) + 1;
> >>>> +list = g_realloc(list, total + len);
> >>>> +memcpy(&list[total], chs_string, len);
> >>>> +total += len;
> >>>> +g_free(chs_string);
> >>>> +g_free(bootpath);
> >>>> +}
&
[PATCH v7 4/8] scsi: Propagate unrealize() callback to scsi-hd
From: Sam Eiderman
We will need to add LCHS removal logic to scsi-hd's unrealize() in the
next commit.
Signed-off-by: Sam Eiderman
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
hw/scsi/scsi-bus.c | 16
include/hw/scsi/scsi.h | 1 +
2 files changed, 17 insertions(+)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index bccb7cc4c6..359d50d6d0 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -59,6 +59,14 @@ static void scsi_device_realize(SCSIDevice *s, Error **errp)
}
}
+static void scsi_device_unrealize(SCSIDevice *s, Error **errp)
+{
+SCSIDeviceClass *sc = SCSI_DEVICE_GET_CLASS(s);
+if (sc->unrealize) {
+sc->unrealize(s, errp);
+}
+}
+
int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
void *hba_private)
{
@@ -217,12 +225,20 @@ static void scsi_qdev_realize(DeviceState *qdev, Error
**errp)
static void scsi_qdev_unrealize(DeviceState *qdev, Error **errp)
{
SCSIDevice *dev = SCSI_DEVICE(qdev);
+Error *local_err = NULL;
if (dev->vmsentry) {
qemu_del_vm_change_state_handler(dev->vmsentry);
}
scsi_device_purge_requests(dev, SENSE_CODE(NO_SENSE));
+
+scsi_device_unrealize(dev, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
blockdev_mark_auto_del(dev->conf.blk);
}
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index d77a92361b..332ef602f4 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -59,6 +59,7 @@ struct SCSIRequest {
typedef struct SCSIDeviceClass {
DeviceClass parent_class;
void (*realize)(SCSIDevice *dev, Error **errp);
+void (*unrealize)(SCSIDevice *dev, Error **errp);
int (*parse_cdb)(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
void *hba_private);
SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun,
--
2.23.0.351.gc4317032e6-goog
[PATCH v7 2/8] block: Support providing LCHS from user
From: Sam Eiderman
Add logical geometry variables to BlockConf.
A user can now supply "lcyls", "lheads" & "lsecs" for any HD device
that supports CHS ("cyls", "heads", "secs").
These devices include:
* ide-hd
* scsi-hd
* virtio-blk-pci
In future commits we will use the provided LCHS and pass it to the BIOS
through fw_cfg to be supplied using INT13 routines.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
include/hw/block/block.h | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index fd55a30bca..d7246f3862 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -26,6 +26,7 @@ typedef struct BlockConf {
uint32_t discard_granularity;
/* geometry, not all devices use this */
uint32_t cyls, heads, secs;
+uint32_t lcyls, lheads, lsecs;
OnOffAuto wce;
bool share_rw;
BlockdevOnError rerror;
@@ -65,7 +66,10 @@ static inline unsigned int get_physical_block_exp(BlockConf
*conf)
#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \
DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \
DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0),\
-DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0)
+DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0), \
+DEFINE_PROP_UINT32("lcyls", _state, _conf.lcyls, 0),\
+DEFINE_PROP_UINT32("lheads", _state, _conf.lheads, 0), \
+DEFINE_PROP_UINT32("lsecs", _state, _conf.lsecs, 0)
#define DEFINE_BLOCK_ERROR_PROPERTIES(_state, _conf)\
DEFINE_PROP_BLOCKDEV_ON_ERROR("rerror", _state, _conf.rerror, \
--
2.23.0.351.gc4317032e6-goog
[PATCH v7 6/8] bootdevice: Refactor get_boot_devices_list
From: Sam Eiderman
Move device name construction to a separate function.
We will reuse this function in the following commit to pass logical CHS
parameters through fw_cfg much like we currently pass bootindex.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
bootdevice.c | 61 +---
1 file changed, 34 insertions(+), 27 deletions(-)
diff --git a/bootdevice.c b/bootdevice.c
index bc5e1c2de4..2b12fb85a4 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -202,6 +202,39 @@ DeviceState *get_boot_device(uint32_t position)
return res;
}
+static char *get_boot_device_path(DeviceState *dev, bool ignore_suffixes,
+ char *suffix)
+{
+char *devpath = NULL, *s = NULL, *d, *bootpath;
+
+if (dev) {
+devpath = qdev_get_fw_dev_path(dev);
+assert(devpath);
+}
+
+if (!ignore_suffixes) {
+if (dev) {
+d = qdev_get_own_fw_dev_path_from_handler(dev->parent_bus, dev);
+if (d) {
+assert(!suffix);
+s = d;
+} else {
+s = g_strdup(suffix);
+}
+} else {
+s = g_strdup(suffix);
+}
+}
+
+bootpath = g_strdup_printf("%s%s",
+ devpath ? devpath : "",
+ s ? s : "");
+g_free(devpath);
+g_free(s);
+
+return bootpath;
+}
+
/*
* This function returns null terminated string that consist of new line
* separated device paths.
@@ -218,36 +251,10 @@ char *get_boot_devices_list(size_t *size)
bool ignore_suffixes = mc->ignore_boot_device_suffixes;
QTAILQ_FOREACH(i, &fw_boot_order, link) {
-char *devpath = NULL, *suffix = NULL;
char *bootpath;
-char *d;
size_t len;
-if (i->dev) {
-devpath = qdev_get_fw_dev_path(i->dev);
-assert(devpath);
-}
-
-if (!ignore_suffixes) {
-if (i->dev) {
-d = qdev_get_own_fw_dev_path_from_handler(i->dev->parent_bus,
- i->dev);
-if (d) {
-assert(!i->suffix);
-suffix = d;
-} else {
-suffix = g_strdup(i->suffix);
-}
-} else {
-suffix = g_strdup(i->suffix);
-}
-}
-
-bootpath = g_strdup_printf("%s%s",
- devpath ? devpath : "",
- suffix ? suffix : "");
-g_free(devpath);
-g_free(suffix);
+bootpath = get_boot_device_path(i->dev, ignore_suffixes, i->suffix);
if (total) {
list[total-1] = '\n';
--
2.23.0.351.gc4317032e6-goog
[PATCH v7 7/8] bootdevice: FW_CFG interface for LCHS values
From: Sam Eiderman
Using fw_cfg, supply logical CHS values directly from QEMU to the BIOS.
Non-standard logical geometries break under QEMU.
A virtual disk which contains an operating system which depends on
logical geometries (consistent values being reported from BIOS INT13
AH=08) will most likely break under QEMU/SeaBIOS if it has non-standard
logical geometries - for example 56 SPT (sectors per track).
No matter what QEMU will report - SeaBIOS, for large enough disks - will
use LBA translation, which will report 63 SPT instead.
In addition we cannot force SeaBIOS to rely on physical geometries at
all. A virtio-blk-pci virtual disk with 255 phyiscal heads cannot
report more than 16 physical heads when moved to an IDE controller,
since the ATA spec allows a maximum of 16 heads - this is an artifact of
virtualization.
By supplying the logical geometries directly we are able to support such
"exotic" disks.
We serialize this information in a similar way to the "bootorder"
interface.
The new fw_cfg entry is "bios-geometry".
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
bootdevice.c| 32
hw/nvram/fw_cfg.c | 14 +++---
include/sysemu/sysemu.h | 1 +
3 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/bootdevice.c b/bootdevice.c
index 2b12fb85a4..b034ad7bdc 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -405,3 +405,35 @@ void del_boot_device_lchs(DeviceState *dev, const char
*suffix)
}
}
}
+
+/* Serialized as: (device name\0 + lchs struct) x devices */
+char *get_boot_devices_lchs_list(size_t *size)
+{
+FWLCHSEntry *i;
+size_t total = 0;
+char *list = NULL;
+
+QTAILQ_FOREACH(i, &fw_lchs, link) {
+char *bootpath;
+char *chs_string;
+size_t len;
+
+bootpath = get_boot_device_path(i->dev, false, i->suffix);
+chs_string = g_strdup_printf("%s %" PRIu32 " %" PRIu32 " %" PRIu32,
+ bootpath, i->lcyls, i->lheads, i->lsecs);
+
+if (total) {
+list[total - 1] = '\n';
+}
+len = strlen(chs_string) + 1;
+list = g_realloc(list, total + len);
+memcpy(&list[total], chs_string, len);
+total += len;
+g_free(chs_string);
+g_free(bootpath);
+}
+
+*size = total;
+
+return list;
+}
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 7dc3ac378e..18aff658c0 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -920,13 +920,21 @@ void *fw_cfg_modify_file(FWCfgState *s, const char
*filename,
static void fw_cfg_machine_reset(void *opaque)
{
+MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
+FWCfgState *s = opaque;
void *ptr;
size_t len;
-FWCfgState *s = opaque;
-char *bootindex = get_boot_devices_list(&len);
+char *buf;
-ptr = fw_cfg_modify_file(s, "bootorder", (uint8_t *)bootindex, len);
+buf = get_boot_devices_list(&len);
+ptr = fw_cfg_modify_file(s, "bootorder", (uint8_t *)buf, len);
g_free(ptr);
+
+if (!mc->legacy_fw_cfg_order) {
+buf = get_boot_devices_lchs_list(&len);
+ptr = fw_cfg_modify_file(s, "bios-geometry", (uint8_t *)buf, len);
+g_free(ptr);
+}
}
static void fw_cfg_machine_ready(struct Notifier *n, void *data)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 5bc5c79cbc..80c57fdc4e 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -106,6 +106,7 @@ void validate_bootdevices(const char *devices, Error
**errp);
void add_boot_device_lchs(DeviceState *dev, const char *suffix,
uint32_t lcyls, uint32_t lheads, uint32_t lsecs);
void del_boot_device_lchs(DeviceState *dev, const char *suffix);
+char *get_boot_devices_lchs_list(size_t *size);
/* handler to set the boot_device order for a specific type of MachineClass */
typedef void QEMUBootSetHandler(void *opaque, const char *boot_order,
--
2.23.0.351.gc4317032e6-goog
[PATCH v7 8/8] hd-geo-test: Add tests for lchs override
From: Sam Eiderman
Add QTest tests to check the logical geometry override option.
The tests in hd-geo-test are out of date - they only test IDE and do not
test interesting MBRs.
I added a few helper functions which will make adding more tests easier.
QTest's fw_cfg helper functions support only legacy fw_cfg, so I had to
read the new fw_cfg layout on my own.
Creating qcow2 disks with specific size and MBR layout is currently
unused - we only use a default empty MBR.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
tests/Makefile.include | 2 +-
tests/hd-geo-test.c| 589 +
2 files changed, 590 insertions(+), 1 deletion(-)
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 479664f899..a5b92fea6a 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -780,7 +780,7 @@ tests/ide-test$(EXESUF): tests/ide-test.o $(libqos-pc-obj-y)
tests/ahci-test$(EXESUF): tests/ahci-test.o $(libqos-pc-obj-y)
qemu-img$(EXESUF)
tests/ipmi-kcs-test$(EXESUF): tests/ipmi-kcs-test.o
tests/ipmi-bt-test$(EXESUF): tests/ipmi-bt-test.o
-tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o
+tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o $(libqos-obj-y)
tests/boot-order-test$(EXESUF): tests/boot-order-test.o $(libqos-obj-y)
tests/boot-serial-test$(EXESUF): tests/boot-serial-test.o $(libqos-obj-y)
tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
index 62eb624726..458de99c31 100644
--- a/tests/hd-geo-test.c
+++ b/tests/hd-geo-test.c
@@ -17,7 +17,12 @@
#include "qemu/osdep.h"
#include "qemu-common.h"
+#include "qemu/bswap.h"
+#include "qapi/qmp/qlist.h"
#include "libqtest.h"
+#include "libqos/fw_cfg.h"
+#include "libqos/libqos.h"
+#include "standard-headers/linux/qemu_fw_cfg.h"
#define ARGV_SIZE 256
@@ -388,6 +393,575 @@ static void test_ide_drive_cd_0(void)
qtest_quit(qts);
}
+typedef struct {
+bool active;
+uint32_t head;
+uint32_t sector;
+uint32_t cyl;
+uint32_t end_head;
+uint32_t end_sector;
+uint32_t end_cyl;
+uint32_t start_sect;
+uint32_t nr_sects;
+} MBRpartitions[4];
+
+static MBRpartitions empty_mbr = { {false, 0, 0, 0, 0, 0, 0, 0, 0},
+ {false, 0, 0, 0, 0, 0, 0, 0, 0},
+ {false, 0, 0, 0, 0, 0, 0, 0, 0},
+ {false, 0, 0, 0, 0, 0, 0, 0, 0} };
+
+static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
+{
+const char *template = "/tmp/qtest.XX";
+char *raw_path = strdup(template);
+char *qcow2_path = strdup(template);
+char cmd[100 + 2 * PATH_MAX];
+uint8_t buf[512];
+int i, ret, fd, offset;
+uint64_t qcow2_size = sectors * 512;
+uint8_t status, parttype, head, sector, cyl;
+char *qemu_img_path;
+char *qemu_img_abs_path;
+
+offset = 0xbe;
+
+for (i = 0; i < 4; i++) {
+status = mbr[i].active ? 0x80 : 0x00;
+g_assert(mbr[i].head < 256);
+g_assert(mbr[i].sector < 64);
+g_assert(mbr[i].cyl < 1024);
+head = mbr[i].head;
+sector = mbr[i].sector + ((mbr[i].cyl & 0x300) >> 2);
+cyl = mbr[i].cyl & 0xff;
+
+buf[offset + 0x0] = status;
+buf[offset + 0x1] = head;
+buf[offset + 0x2] = sector;
+buf[offset + 0x3] = cyl;
+
+parttype = 0;
+g_assert(mbr[i].end_head < 256);
+g_assert(mbr[i].end_sector < 64);
+g_assert(mbr[i].end_cyl < 1024);
+head = mbr[i].end_head;
+sector = mbr[i].end_sector + ((mbr[i].end_cyl & 0x300) >> 2);
+cyl = mbr[i].end_cyl & 0xff;
+
+buf[offset + 0x4] = parttype;
+buf[offset + 0x5] = head;
+buf[offset + 0x6] = sector;
+buf[offset + 0x7] = cyl;
+
+(*(uint32_t *)&buf[offset + 0x8]) = cpu_to_le32(mbr[i].start_sect);
+(*(uint32_t *)&buf[offset + 0xc]) = cpu_to_le32(mbr[i].nr_sects);
+
+offset += 0x10;
+}
+
+fd = mkstemp(raw_path);
+g_assert(fd);
+close(fd);
+
+fd = open(raw_path, O_WRONLY);
+g_assert(fd >= 0);
+ret = write(fd, buf, sizeof(buf));
+g_assert(ret == sizeof(buf));
+close(fd);
+
+fd = mkstemp(qcow2_path);
+g_assert(fd);
+close(fd);
+
+qemu_img_path = getenv("QTEST_QEMU_IMG");
+g_assert(qemu_img_path);
+qemu_img_abs_path = realpath(qemu_img_path, NULL);
+g_assert(qemu_img_abs_path);
+
+ret = snprintf(cmd, sizeof(cmd),
+ "%s convert -f raw -O qcow2 %s %s > /dev/null",
+ qemu_img_abs_path,
+ raw_path, qcow2_path);
+g_assert((0 < ret) && (ret <= sizeof(cmd)));
+ret = system(cmd);
+g_assert(
[PATCH v7 0/8] Add Qemu to SeaBIOS LCHS interface
v1: Non-standard logical geometries break under QEMU. A virtual disk which contains an operating system which depends on logical geometries (consistent values being reported from BIOS INT13 AH=08) will most likely break under QEMU/SeaBIOS if it has non-standard logical geometries - for example 56 SPT (sectors per track). No matter what QEMU will guess - SeaBIOS, for large enough disks - will use LBA translation, which will report 63 SPT instead. In addition we can not enforce SeaBIOS to rely on phyiscal geometries at all. A virtio-blk-pci virtual disk with 255 phyiscal heads can not report more than 16 physical heads when moved to an IDE controller, the ATA spec allows a maximum of 16 heads - this is an artifact of virtualization. By supplying the logical geometies directly we are able to support such "exotic" disks. We will use fw_cfg to do just that. v2: Fix missing parenthesis check in "hd-geo-test: Add tests for lchs override" v3: * Rename fw_cfg key to "bios-geometry". * Remove "extendible" interface. * Add cpu_to_le32 fix as Laszlo suggested or big endian hosts * Fix last qtest commit - automatic docker tester for some reason does not have qemu-img set v4: * Change fw_cfg interface from mixed textual/binary to textual only v5: * Fix line > 80 chars in tests/hd-geo-test.c v6: * Small fixes for issues pointed by Max * (&conf->conf)->lcyls to conf->conf.lcyls and so on * Remove scsi_unrealize from everything other than scsi-hd * Add proper include to sysemu.h * scsi_device_unrealize() after scsi_device_purge_requests() v7: * Adapted last commit (tests) to changes in qtest Sam Eiderman (8): block: Refactor macros - fix tabbing block: Support providing LCHS from user bootdevice: Add interface to gather LCHS scsi: Propagate unrealize() callback to scsi-hd bootdevice: Gather LCHS from all relevant devices bootdevice: Refactor get_boot_devices_list bootdevice: FW_CFG interface for LCHS values hd-geo-test: Add tests for lchs override bootdevice.c | 148 -- hw/block/virtio-blk.c| 6 + hw/ide/qdev.c| 7 +- hw/nvram/fw_cfg.c| 14 +- hw/scsi/scsi-bus.c | 16 ++ hw/scsi/scsi-disk.c | 12 + include/hw/block/block.h | 22 +- include/hw/scsi/scsi.h | 1 + include/sysemu/sysemu.h | 4 + tests/Makefile.include | 2 +- tests/hd-geo-test.c | 589 +++ 11 files changed, 780 insertions(+), 41 deletions(-) -- 2.23.0.351.gc4317032e6-goog
[PATCH v7 5/8] bootdevice: Gather LCHS from all relevant devices
From: Sam Eiderman
Relevant devices are:
* ide-hd (and ide-cd, ide-drive)
* scsi-hd (and scsi-cd, scsi-disk, scsi-block)
* virtio-blk-pci
We do not call del_boot_device_lchs() for ide-* since we don't need to -
IDE block devices do not support unplugging.
Signed-off-by: Sam Eiderman
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
hw/block/virtio-blk.c | 6 ++
hw/ide/qdev.c | 5 +
hw/scsi/scsi-disk.c | 12
3 files changed, 23 insertions(+)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 18851601cb..6d8ff34a16 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1186,6 +1186,11 @@ static void virtio_blk_device_realize(DeviceState *dev,
Error **errp)
blk_set_guest_block_size(s->blk, s->conf.conf.logical_block_size);
blk_iostatus_enable(s->blk);
+
+add_boot_device_lchs(dev, "/disk@0,0",
+ conf->conf.lcyls,
+ conf->conf.lheads,
+ conf->conf.lsecs);
}
static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp)
@@ -1193,6 +1198,7 @@ static void virtio_blk_device_unrealize(DeviceState *dev,
Error **errp)
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
VirtIOBlock *s = VIRTIO_BLK(dev);
+del_boot_device_lchs(dev, "/disk@0,0");
virtio_blk_data_plane_destroy(s->dataplane);
s->dataplane = NULL;
qemu_del_vm_change_state_handler(s->change);
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 6dd219944f..2ffd387a73 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -220,6 +220,11 @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind
kind, Error **errp)
add_boot_device_path(dev->conf.bootindex, &dev->qdev,
dev->unit ? "/disk@1" : "/disk@0");
+
+add_boot_device_lchs(&dev->qdev, dev->unit ? "/disk@1" : "/disk@0",
+ dev->conf.lcyls,
+ dev->conf.lheads,
+ dev->conf.lsecs);
}
static void ide_dev_get_bootindex(Object *obj, Visitor *v, const char *name,
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 915641a0f1..d19896fe4d 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -35,6 +35,7 @@
#include "hw/block/block.h"
#include "hw/qdev-properties.h"
#include "sysemu/dma.h"
+#include "sysemu/sysemu.h"
#include "qemu/cutils.h"
#include "trace.h"
@@ -2402,6 +2403,16 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
blk_set_guest_block_size(s->qdev.conf.blk, s->qdev.blocksize);
blk_iostatus_enable(s->qdev.conf.blk);
+
+add_boot_device_lchs(&dev->qdev, NULL,
+ dev->conf.lcyls,
+ dev->conf.lheads,
+ dev->conf.lsecs);
+}
+
+static void scsi_unrealize(SCSIDevice *dev, Error **errp)
+{
+del_boot_device_lchs(&dev->qdev, NULL);
}
static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
@@ -3006,6 +3017,7 @@ static void scsi_hd_class_initfn(ObjectClass *klass, void
*data)
SCSIDeviceClass *sc = SCSI_DEVICE_CLASS(klass);
sc->realize = scsi_hd_realize;
+sc->unrealize= scsi_unrealize;
sc->alloc_req= scsi_new_request;
sc->unit_attention_reported = scsi_disk_unit_attention_reported;
dc->desc = "virtual SCSI disk";
--
2.23.0.351.gc4317032e6-goog
[PATCH v7 3/8] bootdevice: Add interface to gather LCHS
From: Sam Eiderman
Add an interface to provide direct logical CHS values for boot devices.
We will use this interface in the next commits.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
bootdevice.c| 55 +
include/sysemu/sysemu.h | 3 +++
2 files changed, 58 insertions(+)
diff --git a/bootdevice.c b/bootdevice.c
index 1d225202f9..bc5e1c2de4 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -343,3 +343,58 @@ void device_add_bootindex_property(Object *obj, int32_t
*bootindex,
/* initialize devices' bootindex property to -1 */
object_property_set_int(obj, -1, name, NULL);
}
+
+typedef struct FWLCHSEntry FWLCHSEntry;
+
+struct FWLCHSEntry {
+QTAILQ_ENTRY(FWLCHSEntry) link;
+DeviceState *dev;
+char *suffix;
+uint32_t lcyls;
+uint32_t lheads;
+uint32_t lsecs;
+};
+
+static QTAILQ_HEAD(, FWLCHSEntry) fw_lchs =
+QTAILQ_HEAD_INITIALIZER(fw_lchs);
+
+void add_boot_device_lchs(DeviceState *dev, const char *suffix,
+ uint32_t lcyls, uint32_t lheads, uint32_t lsecs)
+{
+FWLCHSEntry *node;
+
+if (!lcyls && !lheads && !lsecs) {
+return;
+}
+
+assert(dev != NULL || suffix != NULL);
+
+node = g_malloc0(sizeof(FWLCHSEntry));
+node->suffix = g_strdup(suffix);
+node->dev = dev;
+node->lcyls = lcyls;
+node->lheads = lheads;
+node->lsecs = lsecs;
+
+QTAILQ_INSERT_TAIL(&fw_lchs, node, link);
+}
+
+void del_boot_device_lchs(DeviceState *dev, const char *suffix)
+{
+FWLCHSEntry *i;
+
+if (dev == NULL) {
+return;
+}
+
+QTAILQ_FOREACH(i, &fw_lchs, link) {
+if ((!suffix || !g_strcmp0(i->suffix, suffix)) &&
+ i->dev == dev) {
+QTAILQ_REMOVE(&fw_lchs, i, link);
+g_free(i->suffix);
+g_free(i);
+
+break;
+}
+}
+}
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 44f18eb739..5bc5c79cbc 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -103,6 +103,9 @@ void device_add_bootindex_property(Object *obj, int32_t
*bootindex,
DeviceState *dev, Error **errp);
void restore_boot_order(void *opaque);
void validate_bootdevices(const char *devices, Error **errp);
+void add_boot_device_lchs(DeviceState *dev, const char *suffix,
+ uint32_t lcyls, uint32_t lheads, uint32_t lsecs);
+void del_boot_device_lchs(DeviceState *dev, const char *suffix);
/* handler to set the boot_device order for a specific type of MachineClass */
typedef void QEMUBootSetHandler(void *opaque, const char *boot_order,
--
2.23.0.351.gc4317032e6-goog
[PATCH v7 1/8] block: Refactor macros - fix tabbing
From: Sam Eiderman
Fixing tabbing in block related macros.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
hw/ide/qdev.c| 2 +-
include/hw/block/block.h | 16
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 6fba6b62b8..6dd219944f 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -290,7 +290,7 @@ static void ide_drive_realize(IDEDevice *dev, Error **errp)
DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf),\
DEFINE_BLOCK_ERROR_PROPERTIES(IDEDrive, dev.conf), \
DEFINE_PROP_STRING("ver", IDEDrive, dev.version), \
-DEFINE_PROP_UINT64("wwn", IDEDrive, dev.wwn, 0),\
+DEFINE_PROP_UINT64("wwn", IDEDrive, dev.wwn, 0), \
DEFINE_PROP_STRING("serial", IDEDrive, dev.serial),\
DEFINE_PROP_STRING("model", IDEDrive, dev.model)
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index 607539057a..fd55a30bca 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -50,21 +50,21 @@ static inline unsigned int get_physical_block_exp(BlockConf
*conf)
_conf.logical_block_size),\
DEFINE_PROP_BLOCKSIZE("physical_block_size", _state,\
_conf.physical_block_size), \
-DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0), \
+DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),\
DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0),\
-DEFINE_PROP_UINT32("discard_granularity", _state, \
- _conf.discard_granularity, -1), \
-DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, \
-ON_OFF_AUTO_AUTO), \
+DEFINE_PROP_UINT32("discard_granularity", _state, \
+ _conf.discard_granularity, -1), \
+DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, \
+ON_OFF_AUTO_AUTO), \
DEFINE_PROP_BOOL("share-rw", _state, _conf.share_rw, false)
#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \
DEFINE_PROP_DRIVE("drive", _state, _conf.blk), \
DEFINE_BLOCK_PROPERTIES_BASE(_state, _conf)
-#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \
-DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \
-DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \
+#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \
+DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \
+DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0),\
DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0)
#define DEFINE_BLOCK_ERROR_PROPERTIES(_state, _conf)\
--
2.23.0.351.gc4317032e6-goog
Re: [Qemu-block] [PATCH v6 0/8] Add Qemu to SeaBIOS LCHS interface
Thanks Thomas, Resubmitting the tests, all other code will remain the same. Sam On Wed, Sep 25, 2019 at 10:58 AM Thomas Huth wrote: > On 24/09/2019 20.49, John Snow wrote: > > Nobody was making movement on this patch series, and in response to Max > > acking the whole series, I was just going to send a pull request for the > > whole thing and see who barked, because nobody likes or hates this > > series enough to offer any feedback. > > > > Unfortunately, it's rotted on the vine a bit and has some conflicts with > > the testing infrastructure now: > > > > /home/jhuston/src/qemu.git/ide/tests/hd-geo-test.c: In function > > ‘test_override’: > > /home/jhuston/src/qemu.git/ide/tests/hd-geo-test.c:732:5: error: > > implicit declaration of function ‘qtest_start’ > > [-Werror=implicit-function-declaration] > > 732 | qtest_start(joined_args); > > > > > > You can jump right to the test by invoking it like this: > > > >> export QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 > >> make tests/hd-geo-test > > > > It looks like some definitions got moved out from under our feet, but > > hopefully it won't take long to rectify. > > Please replace qtest_start() with qts = qtest_init() and qtest_end() > with qtest_quit(qts). > > See this commit for some more details: > > https://git.qemu.org/?p=qemu.git;a=commitdiff;h=44c2364aaa5e366c4 > > Thomas >
Re: [PATCH v6 0/8] Add Qemu to SeaBIOS LCHS interface
Gentle ping On Wed, Sep 11, 2019 at 5:36 PM Sam Eiderman wrote: > > Gentle ping > > On Tue, Aug 27, 2019, 11:24 Sam Eiderman wrote: >> >> v1: >> >> Non-standard logical geometries break under QEMU. >> >> A virtual disk which contains an operating system which depends on >> logical geometries (consistent values being reported from BIOS INT13 >> AH=08) will most likely break under QEMU/SeaBIOS if it has non-standard >> logical geometries - for example 56 SPT (sectors per track). >> No matter what QEMU will guess - SeaBIOS, for large enough disks - will >> use LBA translation, which will report 63 SPT instead. >> >> In addition we can not enforce SeaBIOS to rely on phyiscal geometries at >> all. A virtio-blk-pci virtual disk with 255 phyiscal heads can not >> report more than 16 physical heads when moved to an IDE controller, the >> ATA spec allows a maximum of 16 heads - this is an artifact of >> virtualization. >> >> By supplying the logical geometies directly we are able to support such >> "exotic" disks. >> >> We will use fw_cfg to do just that. >> >> v2: >> >> Fix missing parenthesis check in >> "hd-geo-test: Add tests for lchs override" >> >> v3: >> >> * Rename fw_cfg key to "bios-geometry". >> * Remove "extendible" interface. >> * Add cpu_to_le32 fix as Laszlo suggested or big endian hosts >> * Fix last qtest commit - automatic docker tester for some reason does not >> have qemu-img set >> >> v4: >> >> * Change fw_cfg interface from mixed textual/binary to textual only >> >> v5: >> >> * Fix line > 80 chars in tests/hd-geo-test.c >> >> v6: >> >> * Small fixes for issues pointed by Max >> * (&conf->conf)->lcyls to conf->conf.lcyls and so on >> * Remove scsi_unrealize from everything other than scsi-hd >> * Add proper include to sysemu.h >> * scsi_device_unrealize() after scsi_device_purge_requests() >> >> Sam Eiderman (8): >> block: Refactor macros - fix tabbing >> block: Support providing LCHS from user >> bootdevice: Add interface to gather LCHS >> scsi: Propagate unrealize() callback to scsi-hd >> bootdevice: Gather LCHS from all relevant devices >> bootdevice: Refactor get_boot_devices_list >> bootdevice: FW_CFG interface for LCHS values >> hd-geo-test: Add tests for lchs override >> >> bootdevice.c | 148 -- >> hw/block/virtio-blk.c| 6 + >> hw/ide/qdev.c| 7 +- >> hw/nvram/fw_cfg.c| 14 +- >> hw/scsi/scsi-bus.c | 16 ++ >> hw/scsi/scsi-disk.c | 12 + >> include/hw/block/block.h | 22 +- >> include/hw/scsi/scsi.h | 1 + >> include/sysemu/sysemu.h | 4 + >> tests/Makefile.include | 2 +- >> tests/hd-geo-test.c | 582 +++ >> 11 files changed, 773 insertions(+), 41 deletions(-) >> >> -- >> 2.23.0.187.g17f5b7556c-goog >>
Re: [Qemu-devel] [PATCH v6 0/8] Add Qemu to SeaBIOS LCHS interface
Gentle ping On Tue, Aug 27, 2019, 11:24 Sam Eiderman wrote: > v1: > > Non-standard logical geometries break under QEMU. > > A virtual disk which contains an operating system which depends on > logical geometries (consistent values being reported from BIOS INT13 > AH=08) will most likely break under QEMU/SeaBIOS if it has non-standard > logical geometries - for example 56 SPT (sectors per track). > No matter what QEMU will guess - SeaBIOS, for large enough disks - will > use LBA translation, which will report 63 SPT instead. > > In addition we can not enforce SeaBIOS to rely on phyiscal geometries at > all. A virtio-blk-pci virtual disk with 255 phyiscal heads can not > report more than 16 physical heads when moved to an IDE controller, the > ATA spec allows a maximum of 16 heads - this is an artifact of > virtualization. > > By supplying the logical geometies directly we are able to support such > "exotic" disks. > > We will use fw_cfg to do just that. > > v2: > > Fix missing parenthesis check in > "hd-geo-test: Add tests for lchs override" > > v3: > > * Rename fw_cfg key to "bios-geometry". > * Remove "extendible" interface. > * Add cpu_to_le32 fix as Laszlo suggested or big endian hosts > * Fix last qtest commit - automatic docker tester for some reason does not > have qemu-img set > > v4: > > * Change fw_cfg interface from mixed textual/binary to textual only > > v5: > > * Fix line > 80 chars in tests/hd-geo-test.c > > v6: > > * Small fixes for issues pointed by Max > * (&conf->conf)->lcyls to conf->conf.lcyls and so on > * Remove scsi_unrealize from everything other than scsi-hd > * Add proper include to sysemu.h > * scsi_device_unrealize() after scsi_device_purge_requests() > > Sam Eiderman (8): > block: Refactor macros - fix tabbing > block: Support providing LCHS from user > bootdevice: Add interface to gather LCHS > scsi: Propagate unrealize() callback to scsi-hd > bootdevice: Gather LCHS from all relevant devices > bootdevice: Refactor get_boot_devices_list > bootdevice: FW_CFG interface for LCHS values > hd-geo-test: Add tests for lchs override > > bootdevice.c | 148 -- > hw/block/virtio-blk.c| 6 + > hw/ide/qdev.c| 7 +- > hw/nvram/fw_cfg.c| 14 +- > hw/scsi/scsi-bus.c | 16 ++ > hw/scsi/scsi-disk.c | 12 + > include/hw/block/block.h | 22 +- > include/hw/scsi/scsi.h | 1 + > include/sysemu/sysemu.h | 4 + > tests/Makefile.include | 2 +- > tests/hd-geo-test.c | 582 +++ > 11 files changed, 773 insertions(+), 41 deletions(-) > > -- > 2.23.0.187.g17f5b7556c-goog > >
[Qemu-devel] [PATCH v6 6/8] bootdevice: Refactor get_boot_devices_list
From: Sam Eiderman
Move device name construction to a separate function.
We will reuse this function in the following commit to pass logical CHS
parameters through fw_cfg much like we currently pass bootindex.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
bootdevice.c | 61 +---
1 file changed, 34 insertions(+), 27 deletions(-)
diff --git a/bootdevice.c b/bootdevice.c
index bc5e1c2de4..2b12fb85a4 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -202,6 +202,39 @@ DeviceState *get_boot_device(uint32_t position)
return res;
}
+static char *get_boot_device_path(DeviceState *dev, bool ignore_suffixes,
+ char *suffix)
+{
+char *devpath = NULL, *s = NULL, *d, *bootpath;
+
+if (dev) {
+devpath = qdev_get_fw_dev_path(dev);
+assert(devpath);
+}
+
+if (!ignore_suffixes) {
+if (dev) {
+d = qdev_get_own_fw_dev_path_from_handler(dev->parent_bus, dev);
+if (d) {
+assert(!suffix);
+s = d;
+} else {
+s = g_strdup(suffix);
+}
+} else {
+s = g_strdup(suffix);
+}
+}
+
+bootpath = g_strdup_printf("%s%s",
+ devpath ? devpath : "",
+ s ? s : "");
+g_free(devpath);
+g_free(s);
+
+return bootpath;
+}
+
/*
* This function returns null terminated string that consist of new line
* separated device paths.
@@ -218,36 +251,10 @@ char *get_boot_devices_list(size_t *size)
bool ignore_suffixes = mc->ignore_boot_device_suffixes;
QTAILQ_FOREACH(i, &fw_boot_order, link) {
-char *devpath = NULL, *suffix = NULL;
char *bootpath;
-char *d;
size_t len;
-if (i->dev) {
-devpath = qdev_get_fw_dev_path(i->dev);
-assert(devpath);
-}
-
-if (!ignore_suffixes) {
-if (i->dev) {
-d = qdev_get_own_fw_dev_path_from_handler(i->dev->parent_bus,
- i->dev);
-if (d) {
-assert(!i->suffix);
-suffix = d;
-} else {
-suffix = g_strdup(i->suffix);
-}
-} else {
-suffix = g_strdup(i->suffix);
-}
-}
-
-bootpath = g_strdup_printf("%s%s",
- devpath ? devpath : "",
- suffix ? suffix : "");
-g_free(devpath);
-g_free(suffix);
+bootpath = get_boot_device_path(i->dev, ignore_suffixes, i->suffix);
if (total) {
list[total-1] = '\n';
--
2.23.0.187.g17f5b7556c-goog
[Qemu-devel] [PATCH v6 2/8] block: Support providing LCHS from user
From: Sam Eiderman
Add logical geometry variables to BlockConf.
A user can now supply "lcyls", "lheads" & "lsecs" for any HD device
that supports CHS ("cyls", "heads", "secs").
These devices include:
* ide-hd
* scsi-hd
* virtio-blk-pci
In future commits we will use the provided LCHS and pass it to the BIOS
through fw_cfg to be supplied using INT13 routines.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
include/hw/block/block.h | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index fd55a30bca..d7246f3862 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -26,6 +26,7 @@ typedef struct BlockConf {
uint32_t discard_granularity;
/* geometry, not all devices use this */
uint32_t cyls, heads, secs;
+uint32_t lcyls, lheads, lsecs;
OnOffAuto wce;
bool share_rw;
BlockdevOnError rerror;
@@ -65,7 +66,10 @@ static inline unsigned int get_physical_block_exp(BlockConf
*conf)
#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \
DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \
DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0),\
-DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0)
+DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0), \
+DEFINE_PROP_UINT32("lcyls", _state, _conf.lcyls, 0),\
+DEFINE_PROP_UINT32("lheads", _state, _conf.lheads, 0), \
+DEFINE_PROP_UINT32("lsecs", _state, _conf.lsecs, 0)
#define DEFINE_BLOCK_ERROR_PROPERTIES(_state, _conf)\
DEFINE_PROP_BLOCKDEV_ON_ERROR("rerror", _state, _conf.rerror, \
--
2.23.0.187.g17f5b7556c-goog
[Qemu-devel] [PATCH v6 8/8] hd-geo-test: Add tests for lchs override
From: Sam Eiderman
Add QTest tests to check the logical geometry override option.
The tests in hd-geo-test are out of date - they only test IDE and do not
test interesting MBRs.
I added a few helper functions which will make adding more tests easier.
QTest's fw_cfg helper functions support only legacy fw_cfg, so I had to
read the new fw_cfg layout on my own.
Creating qcow2 disks with specific size and MBR layout is currently
unused - we only use a default empty MBR.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
tests/Makefile.include | 2 +-
tests/hd-geo-test.c| 582 +
2 files changed, 583 insertions(+), 1 deletion(-)
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 39bed753b3..bd385e2150 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -781,7 +781,7 @@ tests/ide-test$(EXESUF): tests/ide-test.o $(libqos-pc-obj-y)
tests/ahci-test$(EXESUF): tests/ahci-test.o $(libqos-pc-obj-y)
qemu-img$(EXESUF)
tests/ipmi-kcs-test$(EXESUF): tests/ipmi-kcs-test.o
tests/ipmi-bt-test$(EXESUF): tests/ipmi-bt-test.o
-tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o
+tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o $(libqos-obj-y)
tests/boot-order-test$(EXESUF): tests/boot-order-test.o $(libqos-obj-y)
tests/boot-serial-test$(EXESUF): tests/boot-serial-test.o $(libqos-obj-y)
tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
index 62eb624726..002f5c4a43 100644
--- a/tests/hd-geo-test.c
+++ b/tests/hd-geo-test.c
@@ -17,7 +17,12 @@
#include "qemu/osdep.h"
#include "qemu-common.h"
+#include "qemu/bswap.h"
+#include "qapi/qmp/qlist.h"
#include "libqtest.h"
+#include "libqos/fw_cfg.h"
+#include "libqos/libqos.h"
+#include "standard-headers/linux/qemu_fw_cfg.h"
#define ARGV_SIZE 256
@@ -388,6 +393,568 @@ static void test_ide_drive_cd_0(void)
qtest_quit(qts);
}
+typedef struct {
+bool active;
+uint32_t head;
+uint32_t sector;
+uint32_t cyl;
+uint32_t end_head;
+uint32_t end_sector;
+uint32_t end_cyl;
+uint32_t start_sect;
+uint32_t nr_sects;
+} MBRpartitions[4];
+
+static MBRpartitions empty_mbr = { {false, 0, 0, 0, 0, 0, 0, 0, 0},
+ {false, 0, 0, 0, 0, 0, 0, 0, 0},
+ {false, 0, 0, 0, 0, 0, 0, 0, 0},
+ {false, 0, 0, 0, 0, 0, 0, 0, 0} };
+
+static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
+{
+const char *template = "/tmp/qtest.XX";
+char *raw_path = strdup(template);
+char *qcow2_path = strdup(template);
+char cmd[100 + 2 * PATH_MAX];
+uint8_t buf[512];
+int i, ret, fd, offset;
+uint64_t qcow2_size = sectors * 512;
+uint8_t status, parttype, head, sector, cyl;
+char *qemu_img_path;
+char *qemu_img_abs_path;
+
+offset = 0xbe;
+
+for (i = 0; i < 4; i++) {
+status = mbr[i].active ? 0x80 : 0x00;
+g_assert(mbr[i].head < 256);
+g_assert(mbr[i].sector < 64);
+g_assert(mbr[i].cyl < 1024);
+head = mbr[i].head;
+sector = mbr[i].sector + ((mbr[i].cyl & 0x300) >> 2);
+cyl = mbr[i].cyl & 0xff;
+
+buf[offset + 0x0] = status;
+buf[offset + 0x1] = head;
+buf[offset + 0x2] = sector;
+buf[offset + 0x3] = cyl;
+
+parttype = 0;
+g_assert(mbr[i].end_head < 256);
+g_assert(mbr[i].end_sector < 64);
+g_assert(mbr[i].end_cyl < 1024);
+head = mbr[i].end_head;
+sector = mbr[i].end_sector + ((mbr[i].end_cyl & 0x300) >> 2);
+cyl = mbr[i].end_cyl & 0xff;
+
+buf[offset + 0x4] = parttype;
+buf[offset + 0x5] = head;
+buf[offset + 0x6] = sector;
+buf[offset + 0x7] = cyl;
+
+(*(uint32_t *)&buf[offset + 0x8]) = cpu_to_le32(mbr[i].start_sect);
+(*(uint32_t *)&buf[offset + 0xc]) = cpu_to_le32(mbr[i].nr_sects);
+
+offset += 0x10;
+}
+
+fd = mkstemp(raw_path);
+g_assert(fd);
+close(fd);
+
+fd = open(raw_path, O_WRONLY);
+g_assert(fd >= 0);
+ret = write(fd, buf, sizeof(buf));
+g_assert(ret == sizeof(buf));
+close(fd);
+
+fd = mkstemp(qcow2_path);
+g_assert(fd);
+close(fd);
+
+qemu_img_path = getenv("QTEST_QEMU_IMG");
+g_assert(qemu_img_path);
+qemu_img_abs_path = realpath(qemu_img_path, NULL);
+g_assert(qemu_img_abs_path);
+
+ret = snprintf(cmd, sizeof(cmd),
+ "%s convert -f raw -O qcow2 %s %s > /dev/null",
+ qemu_img_abs_path,
+ raw_path, qcow2_path);
+g_assert((0 < ret) && (ret <= sizeof(cmd)));
+ret = system(cmd);
+g_assert(
[Qemu-devel] [PATCH v6 4/8] scsi: Propagate unrealize() callback to scsi-hd
From: Sam Eiderman
We will need to add LCHS removal logic to scsi-hd's unrealize() in the
next commit.
Signed-off-by: Sam Eiderman
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
hw/scsi/scsi-bus.c | 16
include/hw/scsi/scsi.h | 1 +
2 files changed, 17 insertions(+)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index bccb7cc4c6..359d50d6d0 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -59,6 +59,14 @@ static void scsi_device_realize(SCSIDevice *s, Error **errp)
}
}
+static void scsi_device_unrealize(SCSIDevice *s, Error **errp)
+{
+SCSIDeviceClass *sc = SCSI_DEVICE_GET_CLASS(s);
+if (sc->unrealize) {
+sc->unrealize(s, errp);
+}
+}
+
int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
void *hba_private)
{
@@ -217,12 +225,20 @@ static void scsi_qdev_realize(DeviceState *qdev, Error
**errp)
static void scsi_qdev_unrealize(DeviceState *qdev, Error **errp)
{
SCSIDevice *dev = SCSI_DEVICE(qdev);
+Error *local_err = NULL;
if (dev->vmsentry) {
qemu_del_vm_change_state_handler(dev->vmsentry);
}
scsi_device_purge_requests(dev, SENSE_CODE(NO_SENSE));
+
+scsi_device_unrealize(dev, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
blockdev_mark_auto_del(dev->conf.blk);
}
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index d77a92361b..332ef602f4 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -59,6 +59,7 @@ struct SCSIRequest {
typedef struct SCSIDeviceClass {
DeviceClass parent_class;
void (*realize)(SCSIDevice *dev, Error **errp);
+void (*unrealize)(SCSIDevice *dev, Error **errp);
int (*parse_cdb)(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
void *hba_private);
SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun,
--
2.23.0.187.g17f5b7556c-goog
[Qemu-devel] [PATCH v6 1/8] block: Refactor macros - fix tabbing
From: Sam Eiderman
Fixing tabbing in block related macros.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
hw/ide/qdev.c| 2 +-
include/hw/block/block.h | 16
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 6fba6b62b8..6dd219944f 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -290,7 +290,7 @@ static void ide_drive_realize(IDEDevice *dev, Error **errp)
DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf),\
DEFINE_BLOCK_ERROR_PROPERTIES(IDEDrive, dev.conf), \
DEFINE_PROP_STRING("ver", IDEDrive, dev.version), \
-DEFINE_PROP_UINT64("wwn", IDEDrive, dev.wwn, 0),\
+DEFINE_PROP_UINT64("wwn", IDEDrive, dev.wwn, 0), \
DEFINE_PROP_STRING("serial", IDEDrive, dev.serial),\
DEFINE_PROP_STRING("model", IDEDrive, dev.model)
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index 607539057a..fd55a30bca 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -50,21 +50,21 @@ static inline unsigned int get_physical_block_exp(BlockConf
*conf)
_conf.logical_block_size),\
DEFINE_PROP_BLOCKSIZE("physical_block_size", _state,\
_conf.physical_block_size), \
-DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0), \
+DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),\
DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0),\
-DEFINE_PROP_UINT32("discard_granularity", _state, \
- _conf.discard_granularity, -1), \
-DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, \
-ON_OFF_AUTO_AUTO), \
+DEFINE_PROP_UINT32("discard_granularity", _state, \
+ _conf.discard_granularity, -1), \
+DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, \
+ON_OFF_AUTO_AUTO), \
DEFINE_PROP_BOOL("share-rw", _state, _conf.share_rw, false)
#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \
DEFINE_PROP_DRIVE("drive", _state, _conf.blk), \
DEFINE_BLOCK_PROPERTIES_BASE(_state, _conf)
-#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \
-DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \
-DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \
+#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \
+DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \
+DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0),\
DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0)
#define DEFINE_BLOCK_ERROR_PROPERTIES(_state, _conf)\
--
2.23.0.187.g17f5b7556c-goog
[Qemu-devel] [PATCH v6 3/8] bootdevice: Add interface to gather LCHS
From: Sam Eiderman
Add an interface to provide direct logical CHS values for boot devices.
We will use this interface in the next commits.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
bootdevice.c| 55 +
include/sysemu/sysemu.h | 3 +++
2 files changed, 58 insertions(+)
diff --git a/bootdevice.c b/bootdevice.c
index 1d225202f9..bc5e1c2de4 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -343,3 +343,58 @@ void device_add_bootindex_property(Object *obj, int32_t
*bootindex,
/* initialize devices' bootindex property to -1 */
object_property_set_int(obj, -1, name, NULL);
}
+
+typedef struct FWLCHSEntry FWLCHSEntry;
+
+struct FWLCHSEntry {
+QTAILQ_ENTRY(FWLCHSEntry) link;
+DeviceState *dev;
+char *suffix;
+uint32_t lcyls;
+uint32_t lheads;
+uint32_t lsecs;
+};
+
+static QTAILQ_HEAD(, FWLCHSEntry) fw_lchs =
+QTAILQ_HEAD_INITIALIZER(fw_lchs);
+
+void add_boot_device_lchs(DeviceState *dev, const char *suffix,
+ uint32_t lcyls, uint32_t lheads, uint32_t lsecs)
+{
+FWLCHSEntry *node;
+
+if (!lcyls && !lheads && !lsecs) {
+return;
+}
+
+assert(dev != NULL || suffix != NULL);
+
+node = g_malloc0(sizeof(FWLCHSEntry));
+node->suffix = g_strdup(suffix);
+node->dev = dev;
+node->lcyls = lcyls;
+node->lheads = lheads;
+node->lsecs = lsecs;
+
+QTAILQ_INSERT_TAIL(&fw_lchs, node, link);
+}
+
+void del_boot_device_lchs(DeviceState *dev, const char *suffix)
+{
+FWLCHSEntry *i;
+
+if (dev == NULL) {
+return;
+}
+
+QTAILQ_FOREACH(i, &fw_lchs, link) {
+if ((!suffix || !g_strcmp0(i->suffix, suffix)) &&
+ i->dev == dev) {
+QTAILQ_REMOVE(&fw_lchs, i, link);
+g_free(i->suffix);
+g_free(i);
+
+break;
+}
+}
+}
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index d2c38f611a..1a33f25a5a 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -105,6 +105,9 @@ void device_add_bootindex_property(Object *obj, int32_t
*bootindex,
DeviceState *dev, Error **errp);
void restore_boot_order(void *opaque);
void validate_bootdevices(const char *devices, Error **errp);
+void add_boot_device_lchs(DeviceState *dev, const char *suffix,
+ uint32_t lcyls, uint32_t lheads, uint32_t lsecs);
+void del_boot_device_lchs(DeviceState *dev, const char *suffix);
/* handler to set the boot_device order for a specific type of MachineClass */
typedef void QEMUBootSetHandler(void *opaque, const char *boot_order,
--
2.23.0.187.g17f5b7556c-goog
[Qemu-devel] [PATCH v6 7/8] bootdevice: FW_CFG interface for LCHS values
From: Sam Eiderman
Using fw_cfg, supply logical CHS values directly from QEMU to the BIOS.
Non-standard logical geometries break under QEMU.
A virtual disk which contains an operating system which depends on
logical geometries (consistent values being reported from BIOS INT13
AH=08) will most likely break under QEMU/SeaBIOS if it has non-standard
logical geometries - for example 56 SPT (sectors per track).
No matter what QEMU will report - SeaBIOS, for large enough disks - will
use LBA translation, which will report 63 SPT instead.
In addition we cannot force SeaBIOS to rely on physical geometries at
all. A virtio-blk-pci virtual disk with 255 phyiscal heads cannot
report more than 16 physical heads when moved to an IDE controller,
since the ATA spec allows a maximum of 16 heads - this is an artifact of
virtualization.
By supplying the logical geometries directly we are able to support such
"exotic" disks.
We serialize this information in a similar way to the "bootorder"
interface.
The new fw_cfg entry is "bios-geometry".
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
bootdevice.c| 32
hw/nvram/fw_cfg.c | 14 +++---
include/sysemu/sysemu.h | 1 +
3 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/bootdevice.c b/bootdevice.c
index 2b12fb85a4..b034ad7bdc 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -405,3 +405,35 @@ void del_boot_device_lchs(DeviceState *dev, const char
*suffix)
}
}
}
+
+/* Serialized as: (device name\0 + lchs struct) x devices */
+char *get_boot_devices_lchs_list(size_t *size)
+{
+FWLCHSEntry *i;
+size_t total = 0;
+char *list = NULL;
+
+QTAILQ_FOREACH(i, &fw_lchs, link) {
+char *bootpath;
+char *chs_string;
+size_t len;
+
+bootpath = get_boot_device_path(i->dev, false, i->suffix);
+chs_string = g_strdup_printf("%s %" PRIu32 " %" PRIu32 " %" PRIu32,
+ bootpath, i->lcyls, i->lheads, i->lsecs);
+
+if (total) {
+list[total - 1] = '\n';
+}
+len = strlen(chs_string) + 1;
+list = g_realloc(list, total + len);
+memcpy(&list[total], chs_string, len);
+total += len;
+g_free(chs_string);
+g_free(bootpath);
+}
+
+*size = total;
+
+return list;
+}
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 7dc3ac378e..18aff658c0 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -920,13 +920,21 @@ void *fw_cfg_modify_file(FWCfgState *s, const char
*filename,
static void fw_cfg_machine_reset(void *opaque)
{
+MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
+FWCfgState *s = opaque;
void *ptr;
size_t len;
-FWCfgState *s = opaque;
-char *bootindex = get_boot_devices_list(&len);
+char *buf;
-ptr = fw_cfg_modify_file(s, "bootorder", (uint8_t *)bootindex, len);
+buf = get_boot_devices_list(&len);
+ptr = fw_cfg_modify_file(s, "bootorder", (uint8_t *)buf, len);
g_free(ptr);
+
+if (!mc->legacy_fw_cfg_order) {
+buf = get_boot_devices_lchs_list(&len);
+ptr = fw_cfg_modify_file(s, "bios-geometry", (uint8_t *)buf, len);
+g_free(ptr);
+}
}
static void fw_cfg_machine_ready(struct Notifier *n, void *data)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 1a33f25a5a..150fe8c0e2 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -108,6 +108,7 @@ void validate_bootdevices(const char *devices, Error
**errp);
void add_boot_device_lchs(DeviceState *dev, const char *suffix,
uint32_t lcyls, uint32_t lheads, uint32_t lsecs);
void del_boot_device_lchs(DeviceState *dev, const char *suffix);
+char *get_boot_devices_lchs_list(size_t *size);
/* handler to set the boot_device order for a specific type of MachineClass */
typedef void QEMUBootSetHandler(void *opaque, const char *boot_order,
--
2.23.0.187.g17f5b7556c-goog
[Qemu-devel] [PATCH v6 5/8] bootdevice: Gather LCHS from all relevant devices
From: Sam Eiderman
Relevant devices are:
* ide-hd (and ide-cd, ide-drive)
* scsi-hd (and scsi-cd, scsi-disk, scsi-block)
* virtio-blk-pci
We do not call del_boot_device_lchs() for ide-* since we don't need to -
IDE block devices do not support unplugging.
Signed-off-by: Sam Eiderman
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
hw/block/virtio-blk.c | 6 ++
hw/ide/qdev.c | 5 +
hw/scsi/scsi-disk.c | 12
3 files changed, 23 insertions(+)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 18851601cb..6d8ff34a16 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1186,6 +1186,11 @@ static void virtio_blk_device_realize(DeviceState *dev,
Error **errp)
blk_set_guest_block_size(s->blk, s->conf.conf.logical_block_size);
blk_iostatus_enable(s->blk);
+
+add_boot_device_lchs(dev, "/disk@0,0",
+ conf->conf.lcyls,
+ conf->conf.lheads,
+ conf->conf.lsecs);
}
static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp)
@@ -1193,6 +1198,7 @@ static void virtio_blk_device_unrealize(DeviceState *dev,
Error **errp)
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
VirtIOBlock *s = VIRTIO_BLK(dev);
+del_boot_device_lchs(dev, "/disk@0,0");
virtio_blk_data_plane_destroy(s->dataplane);
s->dataplane = NULL;
qemu_del_vm_change_state_handler(s->change);
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 6dd219944f..2ffd387a73 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -220,6 +220,11 @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind
kind, Error **errp)
add_boot_device_path(dev->conf.bootindex, &dev->qdev,
dev->unit ? "/disk@1" : "/disk@0");
+
+add_boot_device_lchs(&dev->qdev, dev->unit ? "/disk@1" : "/disk@0",
+ dev->conf.lcyls,
+ dev->conf.lheads,
+ dev->conf.lsecs);
}
static void ide_dev_get_bootindex(Object *obj, Visitor *v, const char *name,
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 915641a0f1..d19896fe4d 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -35,6 +35,7 @@
#include "hw/block/block.h"
#include "hw/qdev-properties.h"
#include "sysemu/dma.h"
+#include "sysemu/sysemu.h"
#include "qemu/cutils.h"
#include "trace.h"
@@ -2402,6 +2403,16 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
blk_set_guest_block_size(s->qdev.conf.blk, s->qdev.blocksize);
blk_iostatus_enable(s->qdev.conf.blk);
+
+add_boot_device_lchs(&dev->qdev, NULL,
+ dev->conf.lcyls,
+ dev->conf.lheads,
+ dev->conf.lsecs);
+}
+
+static void scsi_unrealize(SCSIDevice *dev, Error **errp)
+{
+del_boot_device_lchs(&dev->qdev, NULL);
}
static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
@@ -3006,6 +3017,7 @@ static void scsi_hd_class_initfn(ObjectClass *klass, void
*data)
SCSIDeviceClass *sc = SCSI_DEVICE_CLASS(klass);
sc->realize = scsi_hd_realize;
+sc->unrealize= scsi_unrealize;
sc->alloc_req= scsi_new_request;
sc->unit_attention_reported = scsi_disk_unit_attention_reported;
dc->desc = "virtual SCSI disk";
--
2.23.0.187.g17f5b7556c-goog
[Qemu-devel] [PATCH v6 0/8] Add Qemu to SeaBIOS LCHS interface
v1: Non-standard logical geometries break under QEMU. A virtual disk which contains an operating system which depends on logical geometries (consistent values being reported from BIOS INT13 AH=08) will most likely break under QEMU/SeaBIOS if it has non-standard logical geometries - for example 56 SPT (sectors per track). No matter what QEMU will guess - SeaBIOS, for large enough disks - will use LBA translation, which will report 63 SPT instead. In addition we can not enforce SeaBIOS to rely on phyiscal geometries at all. A virtio-blk-pci virtual disk with 255 phyiscal heads can not report more than 16 physical heads when moved to an IDE controller, the ATA spec allows a maximum of 16 heads - this is an artifact of virtualization. By supplying the logical geometies directly we are able to support such "exotic" disks. We will use fw_cfg to do just that. v2: Fix missing parenthesis check in "hd-geo-test: Add tests for lchs override" v3: * Rename fw_cfg key to "bios-geometry". * Remove "extendible" interface. * Add cpu_to_le32 fix as Laszlo suggested or big endian hosts * Fix last qtest commit - automatic docker tester for some reason does not have qemu-img set v4: * Change fw_cfg interface from mixed textual/binary to textual only v5: * Fix line > 80 chars in tests/hd-geo-test.c v6: * Small fixes for issues pointed by Max * (&conf->conf)->lcyls to conf->conf.lcyls and so on * Remove scsi_unrealize from everything other than scsi-hd * Add proper include to sysemu.h * scsi_device_unrealize() after scsi_device_purge_requests() Sam Eiderman (8): block: Refactor macros - fix tabbing block: Support providing LCHS from user bootdevice: Add interface to gather LCHS scsi: Propagate unrealize() callback to scsi-hd bootdevice: Gather LCHS from all relevant devices bootdevice: Refactor get_boot_devices_list bootdevice: FW_CFG interface for LCHS values hd-geo-test: Add tests for lchs override bootdevice.c | 148 -- hw/block/virtio-blk.c| 6 + hw/ide/qdev.c| 7 +- hw/nvram/fw_cfg.c| 14 +- hw/scsi/scsi-bus.c | 16 ++ hw/scsi/scsi-disk.c | 12 + include/hw/block/block.h | 22 +- include/hw/scsi/scsi.h | 1 + include/sysemu/sysemu.h | 4 + tests/Makefile.include | 2 +- tests/hd-geo-test.c | 582 +++ 11 files changed, 773 insertions(+), 41 deletions(-) -- 2.23.0.187.g17f5b7556c-goog
Re: [Qemu-devel] [QEMU] [PATCH v5 5/8] bootdevice: Gather LCHS from all relevant devices
> Only scsi-hd has the lchs properties, though, so what’s the purpose of > defining the unrealize function for all other classes? > > Max - shmuel.eider...@oracle.com + sam...@google.com The only purpose is to already have them mapped to the correct existing function, in case it will be used later on. I can resubmit without the unrealize for the other classes, WDYT? Sam
Re: [Qemu-devel] [QEMU] [PATCH v5 4/8] scsi: Propagate unrealize() callback to scsi-hd
> @@ -213,11 +221,18 @@ static void scsi_qdev_realize(DeviceState *qdev, Error
> **errp)
> static void scsi_qdev_unrealize(DeviceState *qdev, Error **errp)
> {
> SCSIDevice *dev = SCSI_DEVICE(qdev);
> +Error *local_err = NULL;
>
> if (dev->vmsentry) {
> qemu_del_vm_change_state_handler(dev->vmsentry);
> }
>
> +scsi_device_unrealize(dev, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> +
> scsi_device_purge_requests(dev, SENSE_CODE(NO_SENSE));
(I see this code for the first time, but) I suppose I’d put the
scsi_device_unrealize() after scsi_device_purge_requests().
Max
> blockdev_mark_auto_del(dev->conf.blk);
> }
- shmuel.eider...@oracle.com
+ sam...@google.com
Sure, I'll resubmit
Sam
Re: [Qemu-devel] [QEMU] [PATCH v5 3/8] bootdevice: Add interface to gather LCHS
> I’ve got a couple of “undelivered mail returned to sender” mails for Sam > recently, but anyway... - shmuel.eider...@oracle.com + sam...@google.com > It doesn’t look like any caller actually passes a NULL @dev, so why not > drop the @suffix part? Just copied it from the bootindex implementation. I think the suffix part there was implemented specifically for fdc since the same device can have two suffixes (A and B). This is not relavant here, but I think we still need the suffix to create the device name for seabios to find. Sam
Re: [Qemu-devel] [QEMU] [PATCH v5 0/8] Add Qemu to SeaBIOS LCHS interface
Well, this patch introduces 3 command line parameters (“lcyls”, “lheads”, “lsecs”) to “scsi-hd” “ide-hd” and “virtio-pci-blk” so this somehow has something to do with block. This patch also adds fw_cfg interface to send these parameters to SeaBIOS. "scripts/get_maintainer.pl -f hw/nvram/fw_cfg.c” gives "Philippe Mathieu-Daudé" (supporter:Firmware configur...) Laszlo Ersek (reviewer:Firmware configur...) Gerd Hoffmann (reviewer:Firmware configur…) And this was already Reviewed-by Gerd. How should I proceed? Sam > On 17 Jul 2019, at 22:46, Kevin Wolf wrote: > > Am 17.07.2019 um 21:03 hat Sam Eiderman geschrieben: >> Gentle ping. > > Through which tree is this supposed to go? I feel this is more firmware > interface related than block layer stuff. > > Kevin > >>> On 8 Jul 2019, at 20:30, Sam Eiderman wrote: >>> >>> Thanks Gerd, >>> >>> Gentle ping on this. >>> >>> Sam >>> >>>> On 1 Jul 2019, at 10:41, Gerd Hoffmann wrote: >>>> >>>> On Wed, Jun 26, 2019 at 03:39:40PM +0300, Sam Eiderman wrote: >>>>> v1: >>>>> >>>>> Non-standard logical geometries break under QEMU. >>>>> >>>>> A virtual disk which contains an operating system which depends on >>>>> logical geometries (consistent values being reported from BIOS INT13 >>>>> AH=08) will most likely break under QEMU/SeaBIOS if it has non-standard >>>>> logical geometries - for example 56 SPT (sectors per track). >>>>> No matter what QEMU will guess - SeaBIOS, for large enough disks - will >>>>> use LBA translation, which will report 63 SPT instead. >>>>> >>>>> In addition we can not enforce SeaBIOS to rely on phyiscal geometries at >>>>> all. A virtio-blk-pci virtual disk with 255 phyiscal heads can not >>>>> report more than 16 physical heads when moved to an IDE controller, the >>>>> ATA spec allows a maximum of 16 heads - this is an artifact of >>>>> virtualization. >>>>> >>>>> By supplying the logical geometies directly we are able to support such >>>>> "exotic" disks. >>>>> >>>>> We will use fw_cfg to do just that. >>>>> >>>>> v2: >>>>> >>>>> Fix missing parenthesis check in >>>>> "hd-geo-test: Add tests for lchs override" >>>>> >>>>> v3: >>>>> >>>>> * Rename fw_cfg key to "bios-geometry". >>>>> * Remove "extendible" interface. >>>>> * Add cpu_to_le32 fix as Laszlo suggested or big endian hosts >>>>> * Fix last qtest commit - automatic docker tester for some reason does >>>>> not have qemu-img set >>>>> >>>>> v4: >>>>> >>>>> * Change fw_cfg interface from mixed textual/binary to textual only >>>>> >>>>> v5: >>>>> >>>>> * Fix line > 80 chars in tests/hd-geo-test.c >>>> >>>> Reviewed-by: Gerd Hoffmann >>>> >>>> cheers, >>>> Gerd >>>> >>> >>
Re: [Qemu-devel] [QEMU] [PATCH v5 0/8] Add Qemu to SeaBIOS LCHS interface
Gentle ping. Sam > On 8 Jul 2019, at 20:30, Sam Eiderman wrote: > > Thanks Gerd, > > Gentle ping on this. > > Sam > >> On 1 Jul 2019, at 10:41, Gerd Hoffmann wrote: >> >> On Wed, Jun 26, 2019 at 03:39:40PM +0300, Sam Eiderman wrote: >>> v1: >>> >>> Non-standard logical geometries break under QEMU. >>> >>> A virtual disk which contains an operating system which depends on >>> logical geometries (consistent values being reported from BIOS INT13 >>> AH=08) will most likely break under QEMU/SeaBIOS if it has non-standard >>> logical geometries - for example 56 SPT (sectors per track). >>> No matter what QEMU will guess - SeaBIOS, for large enough disks - will >>> use LBA translation, which will report 63 SPT instead. >>> >>> In addition we can not enforce SeaBIOS to rely on phyiscal geometries at >>> all. A virtio-blk-pci virtual disk with 255 phyiscal heads can not >>> report more than 16 physical heads when moved to an IDE controller, the >>> ATA spec allows a maximum of 16 heads - this is an artifact of >>> virtualization. >>> >>> By supplying the logical geometies directly we are able to support such >>> "exotic" disks. >>> >>> We will use fw_cfg to do just that. >>> >>> v2: >>> >>> Fix missing parenthesis check in >>> "hd-geo-test: Add tests for lchs override" >>> >>> v3: >>> >>> * Rename fw_cfg key to "bios-geometry". >>> * Remove "extendible" interface. >>> * Add cpu_to_le32 fix as Laszlo suggested or big endian hosts >>> * Fix last qtest commit - automatic docker tester for some reason does not >>> have qemu-img set >>> >>> v4: >>> >>> * Change fw_cfg interface from mixed textual/binary to textual only >>> >>> v5: >>> >>> * Fix line > 80 chars in tests/hd-geo-test.c >> >> Reviewed-by: Gerd Hoffmann >> >> cheers, >> Gerd >> >
Re: [Qemu-devel] [QEMU] [PATCH v5 0/8] Add Qemu to SeaBIOS LCHS interface
Thanks Gerd, Gentle ping on this. Sam > On 1 Jul 2019, at 10:41, Gerd Hoffmann wrote: > > On Wed, Jun 26, 2019 at 03:39:40PM +0300, Sam Eiderman wrote: >> v1: >> >> Non-standard logical geometries break under QEMU. >> >> A virtual disk which contains an operating system which depends on >> logical geometries (consistent values being reported from BIOS INT13 >> AH=08) will most likely break under QEMU/SeaBIOS if it has non-standard >> logical geometries - for example 56 SPT (sectors per track). >> No matter what QEMU will guess - SeaBIOS, for large enough disks - will >> use LBA translation, which will report 63 SPT instead. >> >> In addition we can not enforce SeaBIOS to rely on phyiscal geometries at >> all. A virtio-blk-pci virtual disk with 255 phyiscal heads can not >> report more than 16 physical heads when moved to an IDE controller, the >> ATA spec allows a maximum of 16 heads - this is an artifact of >> virtualization. >> >> By supplying the logical geometies directly we are able to support such >> "exotic" disks. >> >> We will use fw_cfg to do just that. >> >> v2: >> >> Fix missing parenthesis check in >>"hd-geo-test: Add tests for lchs override" >> >> v3: >> >> * Rename fw_cfg key to "bios-geometry". >> * Remove "extendible" interface. >> * Add cpu_to_le32 fix as Laszlo suggested or big endian hosts >> * Fix last qtest commit - automatic docker tester for some reason does not >> have qemu-img set >> >> v4: >> >> * Change fw_cfg interface from mixed textual/binary to textual only >> >> v5: >> >> * Fix line > 80 chars in tests/hd-geo-test.c > > Reviewed-by: Gerd Hoffmann > > cheers, > Gerd >
[Qemu-devel] [QEMU] [PATCH v5 8/8] hd-geo-test: Add tests for lchs override
Add QTest tests to check the logical geometry override option.
The tests in hd-geo-test are out of date - they only test IDE and do not
test interesting MBRs.
I added a few helper functions which will make adding more tests easier.
QTest's fw_cfg helper functions support only legacy fw_cfg, so I had to
read the new fw_cfg layout on my own.
Creating qcow2 disks with specific size and MBR layout is currently
unused - we only use a default empty MBR.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
tests/Makefile.include | 2 +-
tests/hd-geo-test.c| 582 +
2 files changed, 583 insertions(+), 1 deletion(-)
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 46a36c2c95..55ea165ed4 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -765,7 +765,7 @@ tests/ide-test$(EXESUF): tests/ide-test.o $(libqos-pc-obj-y)
tests/ahci-test$(EXESUF): tests/ahci-test.o $(libqos-pc-obj-y)
qemu-img$(EXESUF)
tests/ipmi-kcs-test$(EXESUF): tests/ipmi-kcs-test.o
tests/ipmi-bt-test$(EXESUF): tests/ipmi-bt-test.o
-tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o
+tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o $(libqos-obj-y)
tests/boot-order-test$(EXESUF): tests/boot-order-test.o $(libqos-obj-y)
tests/boot-serial-test$(EXESUF): tests/boot-serial-test.o $(libqos-obj-y)
tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
index 62eb624726..002f5c4a43 100644
--- a/tests/hd-geo-test.c
+++ b/tests/hd-geo-test.c
@@ -17,7 +17,12 @@
#include "qemu/osdep.h"
#include "qemu-common.h"
+#include "qemu/bswap.h"
+#include "qapi/qmp/qlist.h"
#include "libqtest.h"
+#include "libqos/fw_cfg.h"
+#include "libqos/libqos.h"
+#include "standard-headers/linux/qemu_fw_cfg.h"
#define ARGV_SIZE 256
@@ -388,6 +393,568 @@ static void test_ide_drive_cd_0(void)
qtest_quit(qts);
}
+typedef struct {
+bool active;
+uint32_t head;
+uint32_t sector;
+uint32_t cyl;
+uint32_t end_head;
+uint32_t end_sector;
+uint32_t end_cyl;
+uint32_t start_sect;
+uint32_t nr_sects;
+} MBRpartitions[4];
+
+static MBRpartitions empty_mbr = { {false, 0, 0, 0, 0, 0, 0, 0, 0},
+ {false, 0, 0, 0, 0, 0, 0, 0, 0},
+ {false, 0, 0, 0, 0, 0, 0, 0, 0},
+ {false, 0, 0, 0, 0, 0, 0, 0, 0} };
+
+static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
+{
+const char *template = "/tmp/qtest.XX";
+char *raw_path = strdup(template);
+char *qcow2_path = strdup(template);
+char cmd[100 + 2 * PATH_MAX];
+uint8_t buf[512];
+int i, ret, fd, offset;
+uint64_t qcow2_size = sectors * 512;
+uint8_t status, parttype, head, sector, cyl;
+char *qemu_img_path;
+char *qemu_img_abs_path;
+
+offset = 0xbe;
+
+for (i = 0; i < 4; i++) {
+status = mbr[i].active ? 0x80 : 0x00;
+g_assert(mbr[i].head < 256);
+g_assert(mbr[i].sector < 64);
+g_assert(mbr[i].cyl < 1024);
+head = mbr[i].head;
+sector = mbr[i].sector + ((mbr[i].cyl & 0x300) >> 2);
+cyl = mbr[i].cyl & 0xff;
+
+buf[offset + 0x0] = status;
+buf[offset + 0x1] = head;
+buf[offset + 0x2] = sector;
+buf[offset + 0x3] = cyl;
+
+parttype = 0;
+g_assert(mbr[i].end_head < 256);
+g_assert(mbr[i].end_sector < 64);
+g_assert(mbr[i].end_cyl < 1024);
+head = mbr[i].end_head;
+sector = mbr[i].end_sector + ((mbr[i].end_cyl & 0x300) >> 2);
+cyl = mbr[i].end_cyl & 0xff;
+
+buf[offset + 0x4] = parttype;
+buf[offset + 0x5] = head;
+buf[offset + 0x6] = sector;
+buf[offset + 0x7] = cyl;
+
+(*(uint32_t *)&buf[offset + 0x8]) = cpu_to_le32(mbr[i].start_sect);
+(*(uint32_t *)&buf[offset + 0xc]) = cpu_to_le32(mbr[i].nr_sects);
+
+offset += 0x10;
+}
+
+fd = mkstemp(raw_path);
+g_assert(fd);
+close(fd);
+
+fd = open(raw_path, O_WRONLY);
+g_assert(fd >= 0);
+ret = write(fd, buf, sizeof(buf));
+g_assert(ret == sizeof(buf));
+close(fd);
+
+fd = mkstemp(qcow2_path);
+g_assert(fd);
+close(fd);
+
+qemu_img_path = getenv("QTEST_QEMU_IMG");
+g_assert(qemu_img_path);
+qemu_img_abs_path = realpath(qemu_img_path, NULL);
+g_assert(qemu_img_abs_path);
+
+ret = snprintf(cmd, sizeof(cmd),
+ "%s convert -f raw -O qcow2 %s %s > /dev/null",
+ qemu_img_abs_path,
+ raw_path, qcow2_path);
+g_assert((0 < ret) && (ret <= sizeof(cmd)));
+ret = system(cmd);
+g_assert(ret == 0);
+
+ret = s
[Qemu-devel] [QEMU] [PATCH v5 7/8] bootdevice: FW_CFG interface for LCHS values
Using fw_cfg, supply logical CHS values directly from QEMU to the BIOS.
Non-standard logical geometries break under QEMU.
A virtual disk which contains an operating system which depends on
logical geometries (consistent values being reported from BIOS INT13
AH=08) will most likely break under QEMU/SeaBIOS if it has non-standard
logical geometries - for example 56 SPT (sectors per track).
No matter what QEMU will report - SeaBIOS, for large enough disks - will
use LBA translation, which will report 63 SPT instead.
In addition we cannot force SeaBIOS to rely on physical geometries at
all. A virtio-blk-pci virtual disk with 255 phyiscal heads cannot
report more than 16 physical heads when moved to an IDE controller,
since the ATA spec allows a maximum of 16 heads - this is an artifact of
virtualization.
By supplying the logical geometries directly we are able to support such
"exotic" disks.
We serialize this information in a similar way to the "bootorder"
interface.
The new fw_cfg entry is "bios-geometry".
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
bootdevice.c| 32
hw/nvram/fw_cfg.c | 14 +++---
include/sysemu/sysemu.h | 1 +
3 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/bootdevice.c b/bootdevice.c
index 2b12fb85a4..b034ad7bdc 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -405,3 +405,35 @@ void del_boot_device_lchs(DeviceState *dev, const char
*suffix)
}
}
}
+
+/* Serialized as: (device name\0 + lchs struct) x devices */
+char *get_boot_devices_lchs_list(size_t *size)
+{
+FWLCHSEntry *i;
+size_t total = 0;
+char *list = NULL;
+
+QTAILQ_FOREACH(i, &fw_lchs, link) {
+char *bootpath;
+char *chs_string;
+size_t len;
+
+bootpath = get_boot_device_path(i->dev, false, i->suffix);
+chs_string = g_strdup_printf("%s %" PRIu32 " %" PRIu32 " %" PRIu32,
+ bootpath, i->lcyls, i->lheads, i->lsecs);
+
+if (total) {
+list[total - 1] = '\n';
+}
+len = strlen(chs_string) + 1;
+list = g_realloc(list, total + len);
+memcpy(&list[total], chs_string, len);
+total += len;
+g_free(chs_string);
+g_free(bootpath);
+}
+
+*size = total;
+
+return list;
+}
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 9f7b7789bc..c1230fe11c 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -916,13 +916,21 @@ void *fw_cfg_modify_file(FWCfgState *s, const char
*filename,
static void fw_cfg_machine_reset(void *opaque)
{
+MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
+FWCfgState *s = opaque;
void *ptr;
size_t len;
-FWCfgState *s = opaque;
-char *bootindex = get_boot_devices_list(&len);
+char *buf;
-ptr = fw_cfg_modify_file(s, "bootorder", (uint8_t *)bootindex, len);
+buf = get_boot_devices_list(&len);
+ptr = fw_cfg_modify_file(s, "bootorder", (uint8_t *)buf, len);
g_free(ptr);
+
+if (!mc->legacy_fw_cfg_order) {
+buf = get_boot_devices_lchs_list(&len);
+ptr = fw_cfg_modify_file(s, "bios-geometry", (uint8_t *)buf, len);
+g_free(ptr);
+}
}
static void fw_cfg_machine_ready(struct Notifier *n, void *data)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 173dfbb539..1ca8799588 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -174,6 +174,7 @@ void validate_bootdevices(const char *devices, Error
**errp);
void add_boot_device_lchs(DeviceState *dev, const char *suffix,
uint32_t lcyls, uint32_t lheads, uint32_t lsecs);
void del_boot_device_lchs(DeviceState *dev, const char *suffix);
+char *get_boot_devices_lchs_list(size_t *size);
/* handler to set the boot_device order for a specific type of MachineClass */
typedef void QEMUBootSetHandler(void *opaque, const char *boot_order,
--
2.13.3
[Qemu-devel] [QEMU] [PATCH v5 5/8] bootdevice: Gather LCHS from all relevant devices
Relevant devices are:
* ide-hd (and ide-cd, ide-drive)
* scsi-hd (and scsi-cd, scsi-disk, scsi-block)
* virtio-blk-pci
We do not call del_boot_device_lchs() for ide-* since we don't need to -
IDE block devices do not support unplugging.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
hw/block/virtio-blk.c | 6 ++
hw/ide/qdev.c | 5 +
hw/scsi/scsi-disk.c | 14 ++
3 files changed, 25 insertions(+)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 06e57a4d39..787bbd768a 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1182,6 +1182,11 @@ static void virtio_blk_device_realize(DeviceState *dev,
Error **errp)
blk_set_guest_block_size(s->blk, s->conf.conf.logical_block_size);
blk_iostatus_enable(s->blk);
+
+add_boot_device_lchs(dev, "/disk@0,0",
+ (&conf->conf)->lcyls,
+ (&conf->conf)->lheads,
+ (&conf->conf)->lsecs);
}
static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp)
@@ -1189,6 +1194,7 @@ static void virtio_blk_device_unrealize(DeviceState *dev,
Error **errp)
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
VirtIOBlock *s = VIRTIO_BLK(dev);
+del_boot_device_lchs(dev, "/disk@0,0");
virtio_blk_data_plane_destroy(s->dataplane);
s->dataplane = NULL;
qemu_del_vm_change_state_handler(s->change);
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 9cae3205df..07f429d5e3 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -215,6 +215,11 @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind
kind, Error **errp)
add_boot_device_path(dev->conf.bootindex, &dev->qdev,
dev->unit ? "/disk@1" : "/disk@0");
+
+add_boot_device_lchs(&dev->qdev, dev->unit ? "/disk@1" : "/disk@0",
+ (&dev->conf)->lcyls,
+ (&dev->conf)->lheads,
+ (&dev->conf)->lsecs);
}
static void ide_dev_get_bootindex(Object *obj, Visitor *v, const char *name,
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 7b89ac798b..3451aefdea 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2390,6 +2390,16 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
blk_set_guest_block_size(s->qdev.conf.blk, s->qdev.blocksize);
blk_iostatus_enable(s->qdev.conf.blk);
+
+add_boot_device_lchs(&dev->qdev, NULL,
+ (&dev->conf)->lcyls,
+ (&dev->conf)->lheads,
+ (&dev->conf)->lsecs);
+}
+
+static void scsi_unrealize(SCSIDevice *dev, Error **errp)
+{
+del_boot_device_lchs(&dev->qdev, NULL);
}
static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
@@ -2988,6 +2998,7 @@ static void scsi_hd_class_initfn(ObjectClass *klass, void
*data)
SCSIDeviceClass *sc = SCSI_DEVICE_CLASS(klass);
sc->realize = scsi_hd_realize;
+sc->unrealize= scsi_unrealize;
sc->alloc_req= scsi_new_request;
sc->unit_attention_reported = scsi_disk_unit_attention_reported;
dc->desc = "virtual SCSI disk";
@@ -3019,6 +3030,7 @@ static void scsi_cd_class_initfn(ObjectClass *klass, void
*data)
SCSIDeviceClass *sc = SCSI_DEVICE_CLASS(klass);
sc->realize = scsi_cd_realize;
+sc->unrealize= scsi_unrealize;
sc->alloc_req= scsi_new_request;
sc->unit_attention_reported = scsi_disk_unit_attention_reported;
dc->desc = "virtual SCSI CD-ROM";
@@ -3054,6 +3066,7 @@ static void scsi_block_class_initfn(ObjectClass *klass,
void *data)
SCSIDiskClass *sdc = SCSI_DISK_BASE_CLASS(klass);
sc->realize = scsi_block_realize;
+sc->unrealize= scsi_unrealize;
sc->alloc_req= scsi_block_new_request;
sc->parse_cdb= scsi_block_parse_cdb;
sdc->dma_readv = scsi_block_dma_readv;
@@ -3095,6 +3108,7 @@ static void scsi_disk_class_initfn(ObjectClass *klass,
void *data)
SCSIDeviceClass *sc = SCSI_DEVICE_CLASS(klass);
sc->realize = scsi_disk_realize;
+sc->unrealize= scsi_unrealize;
sc->alloc_req= scsi_new_request;
sc->unit_attention_reported = scsi_disk_unit_attention_reported;
dc->fw_name = "disk";
--
2.13.3
[Qemu-devel] [QEMU] [PATCH v5 2/8] block: Support providing LCHS from user
Add logical geometry variables to BlockConf.
A user can now supply "lcyls", "lheads" & "lsecs" for any HD device
that supports CHS ("cyls", "heads", "secs").
These devices include:
* ide-hd
* scsi-hd
* virtio-blk-pci
In future commits we will use the provided LCHS and pass it to the BIOS
through fw_cfg to be supplied using INT13 routines.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
include/hw/block/block.h | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index fd55a30bca..d7246f3862 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -26,6 +26,7 @@ typedef struct BlockConf {
uint32_t discard_granularity;
/* geometry, not all devices use this */
uint32_t cyls, heads, secs;
+uint32_t lcyls, lheads, lsecs;
OnOffAuto wce;
bool share_rw;
BlockdevOnError rerror;
@@ -65,7 +66,10 @@ static inline unsigned int get_physical_block_exp(BlockConf
*conf)
#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \
DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \
DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0),\
-DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0)
+DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0), \
+DEFINE_PROP_UINT32("lcyls", _state, _conf.lcyls, 0),\
+DEFINE_PROP_UINT32("lheads", _state, _conf.lheads, 0), \
+DEFINE_PROP_UINT32("lsecs", _state, _conf.lsecs, 0)
#define DEFINE_BLOCK_ERROR_PROPERTIES(_state, _conf)\
DEFINE_PROP_BLOCKDEV_ON_ERROR("rerror", _state, _conf.rerror, \
--
2.13.3
[Qemu-devel] [QEMU] [PATCH v5 6/8] bootdevice: Refactor get_boot_devices_list
Move device name construction to a separate function.
We will reuse this function in the following commit to pass logical CHS
parameters through fw_cfg much like we currently pass bootindex.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
bootdevice.c | 61 +---
1 file changed, 34 insertions(+), 27 deletions(-)
diff --git a/bootdevice.c b/bootdevice.c
index bc5e1c2de4..2b12fb85a4 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -202,6 +202,39 @@ DeviceState *get_boot_device(uint32_t position)
return res;
}
+static char *get_boot_device_path(DeviceState *dev, bool ignore_suffixes,
+ char *suffix)
+{
+char *devpath = NULL, *s = NULL, *d, *bootpath;
+
+if (dev) {
+devpath = qdev_get_fw_dev_path(dev);
+assert(devpath);
+}
+
+if (!ignore_suffixes) {
+if (dev) {
+d = qdev_get_own_fw_dev_path_from_handler(dev->parent_bus, dev);
+if (d) {
+assert(!suffix);
+s = d;
+} else {
+s = g_strdup(suffix);
+}
+} else {
+s = g_strdup(suffix);
+}
+}
+
+bootpath = g_strdup_printf("%s%s",
+ devpath ? devpath : "",
+ s ? s : "");
+g_free(devpath);
+g_free(s);
+
+return bootpath;
+}
+
/*
* This function returns null terminated string that consist of new line
* separated device paths.
@@ -218,36 +251,10 @@ char *get_boot_devices_list(size_t *size)
bool ignore_suffixes = mc->ignore_boot_device_suffixes;
QTAILQ_FOREACH(i, &fw_boot_order, link) {
-char *devpath = NULL, *suffix = NULL;
char *bootpath;
-char *d;
size_t len;
-if (i->dev) {
-devpath = qdev_get_fw_dev_path(i->dev);
-assert(devpath);
-}
-
-if (!ignore_suffixes) {
-if (i->dev) {
-d = qdev_get_own_fw_dev_path_from_handler(i->dev->parent_bus,
- i->dev);
-if (d) {
-assert(!i->suffix);
-suffix = d;
-} else {
-suffix = g_strdup(i->suffix);
-}
-} else {
-suffix = g_strdup(i->suffix);
-}
-}
-
-bootpath = g_strdup_printf("%s%s",
- devpath ? devpath : "",
- suffix ? suffix : "");
-g_free(devpath);
-g_free(suffix);
+bootpath = get_boot_device_path(i->dev, ignore_suffixes, i->suffix);
if (total) {
list[total-1] = '\n';
--
2.13.3
[Qemu-devel] [QEMU] [PATCH v5 0/8] Add Qemu to SeaBIOS LCHS interface
v1: Non-standard logical geometries break under QEMU. A virtual disk which contains an operating system which depends on logical geometries (consistent values being reported from BIOS INT13 AH=08) will most likely break under QEMU/SeaBIOS if it has non-standard logical geometries - for example 56 SPT (sectors per track). No matter what QEMU will guess - SeaBIOS, for large enough disks - will use LBA translation, which will report 63 SPT instead. In addition we can not enforce SeaBIOS to rely on phyiscal geometries at all. A virtio-blk-pci virtual disk with 255 phyiscal heads can not report more than 16 physical heads when moved to an IDE controller, the ATA spec allows a maximum of 16 heads - this is an artifact of virtualization. By supplying the logical geometies directly we are able to support such "exotic" disks. We will use fw_cfg to do just that. v2: Fix missing parenthesis check in "hd-geo-test: Add tests for lchs override" v3: * Rename fw_cfg key to "bios-geometry". * Remove "extendible" interface. * Add cpu_to_le32 fix as Laszlo suggested or big endian hosts * Fix last qtest commit - automatic docker tester for some reason does not have qemu-img set v4: * Change fw_cfg interface from mixed textual/binary to textual only v5: * Fix line > 80 chars in tests/hd-geo-test.c Sam Eiderman (8): block: Refactor macros - fix tabbing block: Support providing LCHS from user bootdevice: Add interface to gather LCHS scsi: Propagate unrealize() callback to scsi-hd bootdevice: Gather LCHS from all relevant devices bootdevice: Refactor get_boot_devices_list bootdevice: FW_CFG interface for LCHS values hd-geo-test: Add tests for lchs override bootdevice.c | 148 +--- hw/block/virtio-blk.c| 6 + hw/ide/qdev.c| 7 +- hw/nvram/fw_cfg.c| 14 +- hw/scsi/scsi-bus.c | 15 ++ hw/scsi/scsi-disk.c | 14 ++ include/hw/block/block.h | 22 +- include/hw/scsi/scsi.h | 1 + include/sysemu/sysemu.h | 4 + tests/Makefile.include | 2 +- tests/hd-geo-test.c | 582 +++ 11 files changed, 774 insertions(+), 41 deletions(-) -- 2.13.3
[Qemu-devel] [QEMU] [PATCH v5 4/8] scsi: Propagate unrealize() callback to scsi-hd
We will need to add LCHS removal logic to scsi-hd's unrealize() in the
next commit.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
hw/scsi/scsi-bus.c | 15 +++
include/hw/scsi/scsi.h | 1 +
2 files changed, 16 insertions(+)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index c480553083..f6fe497a1a 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -55,6 +55,14 @@ static void scsi_device_realize(SCSIDevice *s, Error **errp)
}
}
+static void scsi_device_unrealize(SCSIDevice *s, Error **errp)
+{
+SCSIDeviceClass *sc = SCSI_DEVICE_GET_CLASS(s);
+if (sc->unrealize) {
+sc->unrealize(s, errp);
+}
+}
+
int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
void *hba_private)
{
@@ -213,11 +221,18 @@ static void scsi_qdev_realize(DeviceState *qdev, Error
**errp)
static void scsi_qdev_unrealize(DeviceState *qdev, Error **errp)
{
SCSIDevice *dev = SCSI_DEVICE(qdev);
+Error *local_err = NULL;
if (dev->vmsentry) {
qemu_del_vm_change_state_handler(dev->vmsentry);
}
+scsi_device_unrealize(dev, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
scsi_device_purge_requests(dev, SENSE_CODE(NO_SENSE));
blockdev_mark_auto_del(dev->conf.blk);
}
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 426566a5c6..8cf71f910d 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -59,6 +59,7 @@ struct SCSIRequest {
typedef struct SCSIDeviceClass {
DeviceClass parent_class;
void (*realize)(SCSIDevice *dev, Error **errp);
+void (*unrealize)(SCSIDevice *dev, Error **errp);
int (*parse_cdb)(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
void *hba_private);
SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun,
--
2.13.3
[Qemu-devel] [SeaBIOS] [PATCH v4 5/5] geometry: Apply LCHS values for boot devices
Boot devices which use overriden LCHS values are:
* ata
* ahci
* scsi
* esp
* lsi
* megasas
* mpt
* pvscsi
* virtio
* virtio-blk
We use these values in get_translation() and setup_translation() by
introducing a new translation type: "TRANSLATION_HOST".
We treat this translation as TRANSLATION_NONE in fill_ata_edd(),
although this does not really matter since now the translation between
physical and logical geometry does not exist.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
src/block.c | 18 +-
src/block.h | 1 +
src/hw/ahci.c| 1 +
src/hw/ata.c | 8
src/hw/esp-scsi.c| 2 ++
src/hw/lsi-scsi.c| 2 ++
src/hw/megasas.c | 1 +
src/hw/mpt-scsi.c| 2 ++
src/hw/pvscsi.c | 1 +
src/hw/virtio-blk.c | 2 ++
src/hw/virtio-scsi.c | 2 ++
11 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/src/block.c b/src/block.c
index f73ec18c..1f600b85 100644
--- a/src/block.c
+++ b/src/block.c
@@ -69,9 +69,18 @@ int create_bounce_buf(void)
* Disk geometry translation
/
+static int
+host_lchs_supplied(struct drive_s *drive)
+{
+return (drive->lchs.head <= 255 &&
+drive->lchs.sector > 0 && drive->lchs.sector <= 63);
+}
+
static u8
get_translation(struct drive_s *drive)
{
+if (host_lchs_supplied(drive))
+return TRANSLATION_HOST;
u8 type = drive->type;
if (CONFIG_QEMU && type == DTYPE_ATA) {
// Emulators pass in the translation info via nvram.
@@ -159,6 +168,12 @@ setup_translation(struct drive_s *drive)
break;
}
break;
+case TRANSLATION_HOST:
+desc = "host-supplied";
+cylinders = drive->lchs.cylinder;
+heads = drive->lchs.head;
+spt = drive->lchs.sector;
+break;
}
// clip to 1024 cylinders in lchs
if (cylinders > 1024)
@@ -423,7 +438,8 @@ fill_ata_edd(struct segoff_s edd, struct drive_s *drive_gf)
u16 options = 0;
if (GET_GLOBALFLAT(drive_gf->type) == DTYPE_ATA) {
u8 translation = GET_GLOBALFLAT(drive_gf->translation);
-if (translation != TRANSLATION_NONE) {
+if ((translation != TRANSLATION_NONE) &&
+(translation != TRANSLATION_HOST)) {
options |= 1<<3; // CHS translation
if (translation == TRANSLATION_LBA)
options |= 1<<9;
diff --git a/src/block.h b/src/block.h
index f64e8807..c1b8d734 100644
--- a/src/block.h
+++ b/src/block.h
@@ -90,6 +90,7 @@ struct drive_s {
#define TRANSLATION_LBA 1
#define TRANSLATION_LARGE 2
#define TRANSLATION_RECHS 3
+#define TRANSLATION_HOST 4
#define EXTTYPE_FLOPPY 0
#define EXTTYPE_HD 1
diff --git a/src/hw/ahci.c b/src/hw/ahci.c
index 1746e7a1..97a072a1 100644
--- a/src/hw/ahci.c
+++ b/src/hw/ahci.c
@@ -593,6 +593,7 @@ static int ahci_port_setup(struct ahci_port_s *port)
, ata_extract_version(buffer));
port->prio = bootprio_find_ata_device(ctrl->pci_tmp, pnr, 0);
}
+boot_lchs_find_ata_device(ctrl->pci_tmp, pnr, 0, &(port->drive.lchs));
return 0;
}
diff --git a/src/hw/ata.c b/src/hw/ata.c
index b6e073cf..f788ce71 100644
--- a/src/hw/ata.c
+++ b/src/hw/ata.c
@@ -755,6 +755,10 @@ init_drive_atapi(struct atadrive_s *dummy, u16 *buffer)
int prio = bootprio_find_ata_device(adrive->chan_gf->pci_tmp,
adrive->chan_gf->chanid,
adrive->slave);
+boot_lchs_find_ata_device(adrive->chan_gf->pci_tmp,
+ adrive->chan_gf->chanid,
+ adrive->slave,
+ &(adrive->drive.lchs));
boot_add_cd(&adrive->drive, desc, prio);
}
@@ -805,6 +809,10 @@ init_drive_ata(struct atadrive_s *dummy, u16 *buffer)
int prio = bootprio_find_ata_device(adrive->chan_gf->pci_tmp,
adrive->chan_gf->chanid,
adrive->slave);
+boot_lchs_find_ata_device(adrive->chan_gf->pci_tmp,
+ adrive->chan_gf->chanid,
+ adrive->slave,
+ &(adrive->drive.lchs));
// Register with bcv system.
boot_add_hd(&adrive->drive, desc, prio);
diff --git a/src/hw/esp-scsi.c b/src/hw/esp-scsi.c
index ffd86d0f..cc25f227 100644
--- a/src/hw/esp-scsi.c
+++ b/src/hw/esp-scsi.c
@@ -181,6 +181,8 @@ esp_scsi_add_lun(u32 lun, struct drive_s *tmpl_drv)
char *name = znprintf(MAXDESCSIZE, "esp
[Qemu-devel] [QEMU] [PATCH v5 3/8] bootdevice: Add interface to gather LCHS
Add an interface to provide direct logical CHS values for boot devices.
We will use this interface in the next commits.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
bootdevice.c| 55 +
include/sysemu/sysemu.h | 3 +++
2 files changed, 58 insertions(+)
diff --git a/bootdevice.c b/bootdevice.c
index 1d225202f9..bc5e1c2de4 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -343,3 +343,58 @@ void device_add_bootindex_property(Object *obj, int32_t
*bootindex,
/* initialize devices' bootindex property to -1 */
object_property_set_int(obj, -1, name, NULL);
}
+
+typedef struct FWLCHSEntry FWLCHSEntry;
+
+struct FWLCHSEntry {
+QTAILQ_ENTRY(FWLCHSEntry) link;
+DeviceState *dev;
+char *suffix;
+uint32_t lcyls;
+uint32_t lheads;
+uint32_t lsecs;
+};
+
+static QTAILQ_HEAD(, FWLCHSEntry) fw_lchs =
+QTAILQ_HEAD_INITIALIZER(fw_lchs);
+
+void add_boot_device_lchs(DeviceState *dev, const char *suffix,
+ uint32_t lcyls, uint32_t lheads, uint32_t lsecs)
+{
+FWLCHSEntry *node;
+
+if (!lcyls && !lheads && !lsecs) {
+return;
+}
+
+assert(dev != NULL || suffix != NULL);
+
+node = g_malloc0(sizeof(FWLCHSEntry));
+node->suffix = g_strdup(suffix);
+node->dev = dev;
+node->lcyls = lcyls;
+node->lheads = lheads;
+node->lsecs = lsecs;
+
+QTAILQ_INSERT_TAIL(&fw_lchs, node, link);
+}
+
+void del_boot_device_lchs(DeviceState *dev, const char *suffix)
+{
+FWLCHSEntry *i;
+
+if (dev == NULL) {
+return;
+}
+
+QTAILQ_FOREACH(i, &fw_lchs, link) {
+if ((!suffix || !g_strcmp0(i->suffix, suffix)) &&
+ i->dev == dev) {
+QTAILQ_REMOVE(&fw_lchs, i, link);
+g_free(i->suffix);
+g_free(i);
+
+break;
+}
+}
+}
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 61579ae71e..173dfbb539 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -171,6 +171,9 @@ void device_add_bootindex_property(Object *obj, int32_t
*bootindex,
DeviceState *dev, Error **errp);
void restore_boot_order(void *opaque);
void validate_bootdevices(const char *devices, Error **errp);
+void add_boot_device_lchs(DeviceState *dev, const char *suffix,
+ uint32_t lcyls, uint32_t lheads, uint32_t lsecs);
+void del_boot_device_lchs(DeviceState *dev, const char *suffix);
/* handler to set the boot_device order for a specific type of MachineClass */
typedef void QEMUBootSetHandler(void *opaque, const char *boot_order,
--
2.13.3
[Qemu-devel] [SeaBIOS] [PATCH v4 4/5] geometry: Add boot_lchs_find_*() utility functions
Adding the following utility functions:
* boot_lchs_find_pci_device
* boot_lchs_find_scsi_device
* boot_lchs_find_ata_device
These will be used to apply LCHS values received through fw_cfg.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
src/Kconfig | 7 +++
src/boot.c | 69 +
src/util.h | 6 ++
3 files changed, 82 insertions(+)
diff --git a/src/Kconfig b/src/Kconfig
index 55a87cb7..6606ce4d 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -72,6 +72,13 @@ endchoice
help
Support controlling of the boot order via the fw_cfg/CBFS
"bootorder" file.
+config HOST_BIOS_GEOMETRY
+depends on BOOT
+bool "Boot device bios geometry override"
+default y
+help
+Support overriding bios (logical) geometry of boot devices via the
+fw_cfg/CBFS "bios-geometry" file.
config COREBOOT_FLASH
depends on COREBOOT
diff --git a/src/boot.c b/src/boot.c
index 52e1c8e6..ea18194d 100644
--- a/src/boot.c
+++ b/src/boot.c
@@ -129,6 +129,8 @@ parse_u32(char *cur, u32 *n)
static void
loadBiosGeometry(void)
{
+if (!CONFIG_HOST_BIOS_GEOMETRY)
+return;
char *f = romfile_loadfile("bios-geometry", NULL);
if (!f)
return;
@@ -168,6 +170,73 @@ loadBiosGeometry(void)
} while (f);
}
+// Search the bios-geometry list for the given glob pattern.
+static BootDeviceLCHS *
+boot_lchs_find(const char *glob)
+{
+dprintf(1, "Searching bios-geometry for: %s\n", glob);
+int i;
+for (i = 0; i < BiosGeometryCount; i++)
+if (glob_prefix(glob, BiosGeometry[i].name))
+return &BiosGeometry[i];
+return NULL;
+}
+
+int boot_lchs_find_pci_device(struct pci_device *pci, struct chs_s *chs)
+{
+if (!CONFIG_HOST_BIOS_GEOMETRY)
+return -1;
+char desc[256];
+build_pci_path(desc, sizeof(desc), "*", pci);
+BootDeviceLCHS *b = boot_lchs_find(desc);
+if (!b)
+return -1;
+chs->cylinder = (u16)b->lcyls;
+chs->head = (u16)b->lheads;
+chs->sector = (u16)b->lsecs;
+return 0;
+}
+
+int boot_lchs_find_scsi_device(struct pci_device *pci, int target, int lun,
+ struct chs_s *chs)
+{
+if (!CONFIG_HOST_BIOS_GEOMETRY)
+return -1;
+if (!pci)
+// support only pci machine for now
+return -1;
+// Find scsi drive - for example: /pci@i0cf8/scsi@5/channel@0/disk@1,0
+char desc[256];
+build_scsi_path(desc, sizeof(desc), pci, target, lun);
+BootDeviceLCHS *b = boot_lchs_find(desc);
+if (!b)
+return -1;
+chs->cylinder = (u16)b->lcyls;
+chs->head = (u16)b->lheads;
+chs->sector = (u16)b->lsecs;
+return 0;
+}
+
+int boot_lchs_find_ata_device(struct pci_device *pci, int chanid, int slave,
+ struct chs_s *chs)
+{
+if (!CONFIG_HOST_BIOS_GEOMETRY)
+return -1;
+if (!pci)
+// support only pci machine for now
+return -1;
+// Find ata drive - for example: /pci@i0cf8/ide@1,1/drive@1/disk@0
+char desc[256];
+build_ata_path(desc, sizeof(desc), pci, chanid, slave);
+BootDeviceLCHS *b = boot_lchs_find(desc);
+if (!b)
+return -1;
+chs->cylinder = (u16)b->lcyls;
+chs->head = (u16)b->lheads;
+chs->sector = (u16)b->lsecs;
+return 0;
+}
+
/
* Boot priority ordering
diff --git a/src/util.h b/src/util.h
index e2afc80c..b173fa88 100644
--- a/src/util.h
+++ b/src/util.h
@@ -38,6 +38,12 @@ struct usbdevice_s;
int bootprio_find_usb(struct usbdevice_s *usbdev, int lun);
int get_keystroke_full(int msec);
int get_keystroke(int msec);
+struct chs_s;
+int boot_lchs_find_pci_device(struct pci_device *pci, struct chs_s *chs);
+int boot_lchs_find_scsi_device(struct pci_device *pci, int target, int lun,
+ struct chs_s *chs);
+int boot_lchs_find_ata_device(struct pci_device *pci, int chanid, int slave,
+ struct chs_s *chs);
// bootsplash.c
void enable_vga_console(void);
--
2.13.3
[Qemu-devel] [SeaBIOS] [PATCH v4 1/5] geometry: Read LCHS from fw_cfg
Read bios geometry for boot devices from fw_cfg.
By receiving LCHS values directly from QEMU through fw_cfg we will be
able to support logical geometries which can not be inferred by SeaBIOS
itself.
(For instance: A 8GB virtio-blk hard drive which was originally created
as an IDE and must report LCHS of */32/63 for its operating system to
function will always break under SeaBIOS since a LARGE/LBA translation
will be used, causing the number of reported logical heads to be > 32.)
The only LCHS paravirtual interface available at the moment is for IDE
disks (rtc_read() in get_translation()) and it's limited to a maximum
of 4 disks (this code existed in SeaBIOS's translation function before
SCSI and VirtIO were even introduced).
This is why we create a new interface which allows passing LCHS
information per hdd.
Boot device information is serialized in the following way:
* device_path lcyls lheads lsecs\n
...
* device_path lcyls lheads lsecs\0
Device path is a null terminated string in the "Open Firmware" device
path format, the same path as used in bootorder.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
src/boot.c | 74 ++
1 file changed, 74 insertions(+)
diff --git a/src/boot.c b/src/boot.c
index 5acf94fe..a2cb167c 100644
--- a/src/boot.c
+++ b/src/boot.c
@@ -24,6 +24,79 @@
/
+ * Boot device logical geometry
+ /
+
+typedef struct BootDeviceLCHS {
+char *name;
+u32 lcyls;
+u32 lheads;
+u32 lsecs;
+} BootDeviceLCHS;
+
+static BootDeviceLCHS *BiosGeometry VARVERIFY32INIT;
+static int BiosGeometryCount;
+
+static char *
+parse_u32(char *cur, u32 *n)
+{
+u32 m = 0;
+if (cur) {
+while ('0' <= *cur && *cur <= '9') {
+m = 10 * m + (*cur - '0');
+cur++;
+}
+if (*cur != '\0')
+cur++;
+}
+*n = m;
+return cur;
+}
+
+static void
+loadBiosGeometry(void)
+{
+char *f = romfile_loadfile("bios-geometry", NULL);
+if (!f)
+return;
+
+int i = 0;
+BiosGeometryCount = 1;
+while (f[i]) {
+if (f[i] == '\n')
+BiosGeometryCount++;
+i++;
+}
+BiosGeometry = malloc_tmphigh(BiosGeometryCount * sizeof(BootDeviceLCHS));
+if (!BiosGeometry) {
+warn_noalloc();
+free(f);
+BiosGeometryCount = 0;
+return;
+}
+
+dprintf(1, "bios geometry:\n");
+i = 0;
+do {
+BootDeviceLCHS *d = &BiosGeometry[i];
+d->name = f;
+f = strchr(f, '\n');
+if (f)
+*(f++) = '\0';
+char *chs_values = strchr(d->name, ' ');
+if (chs_values)
+*(chs_values++) = '\0';
+chs_values = parse_u32(chs_values, &d->lcyls);
+chs_values = parse_u32(chs_values, &d->lheads);
+chs_values = parse_u32(chs_values, &d->lsecs);
+dprintf(1, "%s: (%u, %u, %u)\n",
+d->name, d->lcyls, d->lheads, d->lsecs);
+i++;
+} while (f);
+}
+
+
+/
* Boot priority ordering
/
@@ -288,6 +361,7 @@ boot_init(void)
BootRetryTime = romfile_loadint("etc/boot-fail-wait", 60*1000);
loadBootOrder();
+loadBiosGeometry();
}
--
2.13.3
[Qemu-devel] [QEMU] [PATCH v5 1/8] block: Refactor macros - fix tabbing
Fixing tabbing in block related macros.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
hw/ide/qdev.c| 2 +-
include/hw/block/block.h | 16
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 360cd20bd8..9cae3205df 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -285,7 +285,7 @@ static void ide_drive_realize(IDEDevice *dev, Error **errp)
DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf),\
DEFINE_BLOCK_ERROR_PROPERTIES(IDEDrive, dev.conf), \
DEFINE_PROP_STRING("ver", IDEDrive, dev.version), \
-DEFINE_PROP_UINT64("wwn", IDEDrive, dev.wwn, 0),\
+DEFINE_PROP_UINT64("wwn", IDEDrive, dev.wwn, 0), \
DEFINE_PROP_STRING("serial", IDEDrive, dev.serial),\
DEFINE_PROP_STRING("model", IDEDrive, dev.model)
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index 607539057a..fd55a30bca 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -50,21 +50,21 @@ static inline unsigned int get_physical_block_exp(BlockConf
*conf)
_conf.logical_block_size),\
DEFINE_PROP_BLOCKSIZE("physical_block_size", _state,\
_conf.physical_block_size), \
-DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0), \
+DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),\
DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0),\
-DEFINE_PROP_UINT32("discard_granularity", _state, \
- _conf.discard_granularity, -1), \
-DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, \
-ON_OFF_AUTO_AUTO), \
+DEFINE_PROP_UINT32("discard_granularity", _state, \
+ _conf.discard_granularity, -1), \
+DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, \
+ON_OFF_AUTO_AUTO), \
DEFINE_PROP_BOOL("share-rw", _state, _conf.share_rw, false)
#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \
DEFINE_PROP_DRIVE("drive", _state, _conf.blk), \
DEFINE_BLOCK_PROPERTIES_BASE(_state, _conf)
-#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \
-DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \
-DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \
+#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \
+DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \
+DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0),\
DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0)
#define DEFINE_BLOCK_ERROR_PROPERTIES(_state, _conf)\
--
2.13.3
[Qemu-devel] [SeaBIOS] [PATCH v4 2/5] boot: Reorder functions in boot.c
Currently glob_prefix() and build_pci_path() are under the "Boot
priority ordering" section.
Move them to a new "Helper search functions" section since we will reuse
them in the next commits.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
src/boot.c | 94 --
1 file changed, 49 insertions(+), 45 deletions(-)
diff --git a/src/boot.c b/src/boot.c
index a2cb167c..70f639f4 100644
--- a/src/boot.c
+++ b/src/boot.c
@@ -22,6 +22,55 @@
#include "util.h" // irqtimer_calc
#include "tcgbios.h" // tpm_*
+/
+ * Helper search functions
+ /
+
+// See if 'str' starts with 'glob' - if glob contains an '*' character
+// it will match any number of characters in str that aren't a '/' or
+// the next glob character.
+static char *
+glob_prefix(const char *glob, const char *str)
+{
+for (;;) {
+if (!*glob && (!*str || *str == '/'))
+return (char*)str;
+if (*glob == '*') {
+if (!*str || *str == '/' || *str == glob[1])
+glob++;
+else
+str++;
+continue;
+}
+if (*glob != *str)
+return NULL;
+glob++;
+str++;
+}
+}
+
+#define FW_PCI_DOMAIN "/pci@i0cf8"
+
+static char *
+build_pci_path(char *buf, int max, const char *devname, struct pci_device *pci)
+{
+// Build the string path of a bdf - for example: /pci@i0cf8/isa@1,2
+char *p = buf;
+if (pci->parent) {
+p = build_pci_path(p, max, "pci-bridge", pci->parent);
+} else {
+p += snprintf(p, buf+max-p, "%s", FW_PCI_DOMAIN);
+if (pci->rootbus)
+p += snprintf(p, buf+max-p, ",%x", pci->rootbus);
+}
+
+int dev = pci_bdf_to_dev(pci->bdf), fn = pci_bdf_to_fn(pci->bdf);
+p += snprintf(p, buf+max-p, "/%s@%x", devname, dev);
+if (fn)
+p += snprintf(p, buf+max-p, ",%x", fn);
+return p;
+}
+
/
* Boot device logical geometry
@@ -141,29 +190,6 @@ loadBootOrder(void)
} while (f);
}
-// See if 'str' starts with 'glob' - if glob contains an '*' character
-// it will match any number of characters in str that aren't a '/' or
-// the next glob character.
-static char *
-glob_prefix(const char *glob, const char *str)
-{
-for (;;) {
-if (!*glob && (!*str || *str == '/'))
-return (char*)str;
-if (*glob == '*') {
-if (!*str || *str == '/' || *str == glob[1])
-glob++;
-else
-str++;
-continue;
-}
-if (*glob != *str)
-return NULL;
-glob++;
-str++;
-}
-}
-
// Search the bootorder list for the given glob pattern.
static int
find_prio(const char *glob)
@@ -176,28 +202,6 @@ find_prio(const char *glob)
return -1;
}
-#define FW_PCI_DOMAIN "/pci@i0cf8"
-
-static char *
-build_pci_path(char *buf, int max, const char *devname, struct pci_device *pci)
-{
-// Build the string path of a bdf - for example: /pci@i0cf8/isa@1,2
-char *p = buf;
-if (pci->parent) {
-p = build_pci_path(p, max, "pci-bridge", pci->parent);
-} else {
-p += snprintf(p, buf+max-p, "%s", FW_PCI_DOMAIN);
-if (pci->rootbus)
-p += snprintf(p, buf+max-p, ",%x", pci->rootbus);
-}
-
-int dev = pci_bdf_to_dev(pci->bdf), fn = pci_bdf_to_fn(pci->bdf);
-p += snprintf(p, buf+max-p, "/%s@%x", devname, dev);
-if (fn)
-p += snprintf(p, buf+max-p, ",%x", fn);
-return p;
-}
-
int bootprio_find_pci_device(struct pci_device *pci)
{
if (CONFIG_CSM)
--
2.13.3
[Qemu-devel] [SeaBIOS] [PATCH v4 3/5] boot: Build ata and scsi paths in function
Introduce build_scsi_path() and build_ata_path().
We will reuse these functions in the next commit.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
src/boot.c | 36
1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/src/boot.c b/src/boot.c
index 70f639f4..52e1c8e6 100644
--- a/src/boot.c
+++ b/src/boot.c
@@ -71,6 +71,30 @@ build_pci_path(char *buf, int max, const char *devname,
struct pci_device *pci)
return p;
}
+static char *
+build_scsi_path(char *buf, int max,
+struct pci_device *pci, int target, int lun)
+{
+// Build the string path of a scsi drive - for example:
+// /pci@i0cf8/scsi@5/channel@0/disk@1,0
+char *p;
+p = build_pci_path(buf, max, "*", pci);
+p += snprintf(p, buf+max-p, "/*@0/*@%x,%x", target, lun);
+return p;
+}
+
+static char *
+build_ata_path(char *buf, int max,
+ struct pci_device *pci, int chanid, int slave)
+{
+// Build the string path of an ata drive - for example:
+// /pci@i0cf8/ide@1,1/drive@1/disk@0
+char *p;
+p = build_pci_path(buf, max, "*", pci);
+p += snprintf(p, buf+max-p, "/drive@%x/disk@%x", chanid, slave);
+return p;
+}
+
/
* Boot device logical geometry
@@ -221,10 +245,8 @@ int bootprio_find_scsi_device(struct pci_device *pci, int
target, int lun)
if (!pci)
// support only pci machine for now
return -1;
-// Find scsi drive - for example: /pci@i0cf8/scsi@5/channel@0/disk@1,0
-char desc[256], *p;
-p = build_pci_path(desc, sizeof(desc), "*", pci);
-snprintf(p, desc+sizeof(desc)-p, "/*@0/*@%x,%x", target, lun);
+char desc[256];
+build_scsi_path(desc, sizeof(desc), pci, target, lun);
return find_prio(desc);
}
@@ -237,10 +259,8 @@ int bootprio_find_ata_device(struct pci_device *pci, int
chanid, int slave)
if (!pci)
// support only pci machine for now
return -1;
-// Find ata drive - for example: /pci@i0cf8/ide@1,1/drive@1/disk@0
-char desc[256], *p;
-p = build_pci_path(desc, sizeof(desc), "*", pci);
-snprintf(p, desc+sizeof(desc)-p, "/drive@%x/disk@%x", chanid, slave);
+char desc[256];
+build_ata_path(desc, sizeof(desc), pci, chanid, slave);
return find_prio(desc);
}
--
2.13.3
[Qemu-devel] [SeaBIOS] [PATCH v4 0/5] Add Qemu to SeaBIOS LCHS interface
v1: Non-standard logical geometries break under QEMU. A virtual disk which contains an operating system which depends on logical geometries (consistent values being reported from BIOS INT13 AH=08) will most likely break under QEMU/SeaBIOS if it has non-standard logical geometries - for example 56 SPT (sectors per track). No matter what QEMU will guess - SeaBIOS, for large enough disks - will use LBA translation, which will report 63 SPT instead. In addition we can not enforce SeaBIOS to rely on phyiscal geometries at all. A virtio-blk-pci virtual disk with 255 phyiscal heads can not report more than 16 physical heads when moved to an IDE controller, the ATA spec allows a maximum of 16 heads - this is an artifact of virtualization. By supplying the logical geometies directly we are able to support such "exotic" disks. We will use fw_cfg to do just that. v2: Rename bootdevices fw_cfg key to bios-geoemtry v3: Change fw_cfg interface from mixed binary/textual to textual only Squash commit "config: Add toggle for bootdevice information" v4: * Rename TRANSLATION_MACHINE to TRANSLATION_HOST * Rename "overriden" translation name to "host-supplied" * Rename overriden_lchs_supplied() to host_lchs_supplied() * Move sectors and heads check to host_lchs_supplied() * Reuse duplicate code in boot.c Sam Eiderman (5): geometry: Read LCHS from fw_cfg boot: Reorder functions in boot.c boot: Build ata and scsi paths in function geometry: Add boot_lchs_find_*() utility functions geometry: Apply LCHS values for boot devices src/Kconfig | 7 ++ src/block.c | 18 +++- src/block.h | 1 + src/boot.c | 273 +-- src/hw/ahci.c| 1 + src/hw/ata.c | 8 ++ src/hw/esp-scsi.c| 2 + src/hw/lsi-scsi.c| 2 + src/hw/megasas.c | 1 + src/hw/mpt-scsi.c| 2 + src/hw/pvscsi.c | 1 + src/hw/virtio-blk.c | 2 + src/hw/virtio-scsi.c | 2 + src/util.h | 6 ++ 14 files changed, 272 insertions(+), 54 deletions(-) -- 2.13.3
Re: [Qemu-devel] [SeaBIOS] [PATCH v3 3/4] geometry: Add boot_lchs_find_*() utility functions
Kevin, Rethinking this change (where we construct the device path from outside and call boot_prio_find()), this is pretty tricky to implement since we need to take care of csm_bootprio_ata() and csm_bootprio_pci() which do not work with device path. In addition, bootprio_find_fdc_device bootprio_find_pci_rom bootprio_find_named_rom bootprio_find_usb Will not require this change and this will just result in too much refactoring. Maybe simply introduce build_scsi_path() and build_ata_path() functions and then, for instance, make booprio_find_scsi_device() and boot_lchs_find_scsi_device() call the same build_scsi_path() function, resulting in less code duplication. Sam > On 22 Jun 2019, at 20:33, Sam Eiderman wrote: > > > >> On 22 Jun 2019, at 18:27, Kevin O'Connor wrote: >> >> On Sat, Jun 22, 2019 at 11:51:48AM +0300, Sam Eiderman wrote: >>> But maybe someone wants bootorder but doesn’t want to override legacy disk >>> translations… >>> >>> I’m thinking of maybe adding >>> >>> if (!CONFIG_BOOTORDER || !CONFIG_BIOS_GEOMETRY) >>> return NULL; >> >> That's fine - though it's (!CONFIG_BOOTORDER && !CONFIG_BIOS_GEOMETRY). > > Yes of course, my bad > >> >> FYI, I think BIOS_GEOMETRY is a little confusing - maybe >> CUSTOM_DISK_GEOMETRY. > > The thing is that disk geometry is actually (physical geometry, reported by > the disk controller) and here > bios geometry stands for the geometry reported from bios int13. > Also “bios geometry” === “logical geometry” === “lchs”. > So following previous discussion with Gerd, maybe CONFIG_HOST_BIOS_GEOMETRY > is better? > > Sam > >> >> -Kevin >
Re: [Qemu-devel] [SeaBIOS] [PATCH v3 3/4] geometry: Add boot_lchs_find_*() utility functions
> On 22 Jun 2019, at 18:27, Kevin O'Connor wrote: > > On Sat, Jun 22, 2019 at 11:51:48AM +0300, Sam Eiderman wrote: >> But maybe someone wants bootorder but doesn’t want to override legacy disk >> translations… >> >> I’m thinking of maybe adding >> >> if (!CONFIG_BOOTORDER || !CONFIG_BIOS_GEOMETRY) >>return NULL; > > That's fine - though it's (!CONFIG_BOOTORDER && !CONFIG_BIOS_GEOMETRY). Yes of course, my bad > > FYI, I think BIOS_GEOMETRY is a little confusing - maybe > CUSTOM_DISK_GEOMETRY. The thing is that disk geometry is actually (physical geometry, reported by the disk controller) and here bios geometry stands for the geometry reported from bios int13. Also “bios geometry” === “logical geometry” === “lchs”. So following previous discussion with Gerd, maybe CONFIG_HOST_BIOS_GEOMETRY is better? Sam > > -Kevin
Re: [Qemu-devel] [SeaBIOS] [PATCH v3 3/4] geometry: Add boot_lchs_find_*() utility functions
But maybe someone wants bootorder but doesn’t want to override legacy disk translations… I’m thinking of maybe adding if (!CONFIG_BOOTORDER || !CONFIG_BIOS_GEOMETRY) return NULL; In each of the get_*_devpath functions (which will normally return an allocated string, not on stack). Another approach can be make CONFIG_BIOS_GEOMETRY depend on CONFIG_BOOTORDER. Then we should only keep: if (!CONFIG_BOOTORDER) return NULL; In the get_*_devpath functions. I think the first approach will look better when reading the code - will not require the reader to analize dependancies in the Kconfig file. Sam > On 21 Jun 2019, at 21:59, Kevin O'Connor wrote: > > On Fri, Jun 21, 2019 at 08:42:28PM +0300, Sam Eiderman wrote: >> Sounds reasonable, how do purpose to deal with: >> >> config BIOS_GEOMETRY >> config BOOTORDER >> >> precompiler optouts? > > I think you can stick them both under BOOTORDER. That option is only > there in case someone wants to reduce the size of the SeaBIOS binary. > I can't think of a reasonable situation where one cares that much > about binary size, yet wants to override legacy disk translations.. > >> If we don’t need any of them we also don’t need to call “get_scsi_devpath", >> “get_ata_devpath”, “get_pci_dev_path”. >> >> I’ll see what can be done. > > Thanks. > -Kevin
Re: [Qemu-devel] [SeaBIOS] [PATCH v3 3/4] geometry: Add boot_lchs_find_*() utility functions
Sounds reasonable, how do purpose to deal with: config BIOS_GEOMETRY config BOOTORDER precompiler optouts? If we don’t need any of them we also don’t need to call “get_scsi_devpath", “get_ata_devpath”, “get_pci_dev_path”. I’ll see what can be done. > On 20 Jun 2019, at 17:37, Kevin O'Connor wrote: > > On Wed, Jun 19, 2019 at 12:23:51PM +0300, Sam Eiderman wrote: >> Adding the following utility functions: >> >>* boot_lchs_find_pci_device >>* boot_lchs_find_scsi_device >>* boot_lchs_find_ata_device > > FWIW, this leads to a bit of code duplication. I think it would be > preferable to refactor the bootprio_find_XYZ() calls. Instead of > returning an 'int prio' they could return a znprintf'd 'char *devpath' > instead. Then the boot_add_XYZ() calls could directly call > find_prio(devpath). The boot_add_hd() could then directly populate > drive->lchs or call setup_translation(). > > -Kevin
[Qemu-devel] [PATCH v3 0/3] vmdk: Add read-only support for the new seSparse format
v1: VMware introduced a new snapshot format in VMFS6 - seSparse (Space Efficient Sparse) which is the default format available in ESXi 6.7. Add read-only support for the new snapshot format. v2: Fixed after Max's review: * Removed strict sesparse checks * Reduced maximal L1 table size * Added non-write mode check in vmdk_open() on sesparse v3: Change non-write mode check in vmdk_open() to bdrv_apply_auto_read_only(). Sam Eiderman (3): vmdk: Fix comment regarding max l1_size coverage vmdk: Reduce the max bound for L1 table size vmdk: Add read-only support for seSparse snapshots block/vmdk.c | 372 ++--- tests/qemu-iotests/059.out | 2 +- 2 files changed, 353 insertions(+), 21 deletions(-) -- 2.13.3
Re: [Qemu-devel] [SeaBIOS] [QEMU] [PATCH v4 0/8] Add Qemu to SeaBIOS LCHS interface
> On 20 Jun 2019, at 8:50, Gerd Hoffmann wrote:
>
>> === OUTPUT BEGIN ===
>> 1/8 Checking commit cf2168a66dbd (block: Refactor macros - fix tabbing)
>> ERROR: Macros with complex values should be enclosed in parenthesis
>> #55: FILE: include/hw/block/block.h:65:
>> +#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \
>> +DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \
>> +DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0),\
>> DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0)
>
> False positive.
>
>> 8/8 Checking commit 6664d7d898d5 (hd-geo-test: Add tests for lchs override)
>> WARNING: line over 80 characters
>> #242: FILE: tests/hd-geo-test.c:579:
>> + (char *)r->dev_path, &(r->chs.c), &(r->chs.h),
>> &(r->chs.s));
>
> Should be fixed.
I’ll resubmit.
Sam
>
>> WARNING: Block comments use a leading /* on a separate line
>> #640: FILE: tests/hd-geo-test.c:995:
>> + "skipping hd-geo/override/* tests");
>
> False positive too.
>
> cheers,
> Gerd
>
[Qemu-devel] [PATCH v3 3/3] vmdk: Add read-only support for seSparse snapshots
0x3:
The grain is allocated - to get the index calculate:
((entry & 0x0fff) >> 48) |
((entry & 0x) << 12)
* The difference between 0x1 and 0x2 is that 0x1 is an unallocated
grain which results from the guest using sg_unmap to unmap the
grain - but the grain itself still exists in the grain extent - a
space reclamation procedure should delete it.
Unmapping a zero grain has no effect (0x2 will not change to 0x1)
but unmapping an unallocated grain will (0x0 to 0x1) - naturally.
In order to implement seSparse some fields had to be changed to support
both 32-bit and 64-bit entry sizes.
Reviewed-by: Karl Heubaum
Reviewed-by: Eyal Moscovici
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
block/vmdk.c | 358 ---
1 file changed, 342 insertions(+), 16 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 931eb2759c..bd36ece125 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -91,6 +91,44 @@ typedef struct {
uint16_t compressAlgorithm;
} QEMU_PACKED VMDK4Header;
+typedef struct VMDKSESparseConstHeader {
+uint64_t magic;
+uint64_t version;
+uint64_t capacity;
+uint64_t grain_size;
+uint64_t grain_table_size;
+uint64_t flags;
+uint64_t reserved1;
+uint64_t reserved2;
+uint64_t reserved3;
+uint64_t reserved4;
+uint64_t volatile_header_offset;
+uint64_t volatile_header_size;
+uint64_t journal_header_offset;
+uint64_t journal_header_size;
+uint64_t journal_offset;
+uint64_t journal_size;
+uint64_t grain_dir_offset;
+uint64_t grain_dir_size;
+uint64_t grain_tables_offset;
+uint64_t grain_tables_size;
+uint64_t free_bitmap_offset;
+uint64_t free_bitmap_size;
+uint64_t backmap_offset;
+uint64_t backmap_size;
+uint64_t grains_offset;
+uint64_t grains_size;
+uint8_t pad[304];
+} QEMU_PACKED VMDKSESparseConstHeader;
+
+typedef struct VMDKSESparseVolatileHeader {
+uint64_t magic;
+uint64_t free_gt_number;
+uint64_t next_txn_seq_number;
+uint64_t replay_journal;
+uint8_t pad[480];
+} QEMU_PACKED VMDKSESparseVolatileHeader;
+
#define L2_CACHE_SIZE 16
typedef struct VmdkExtent {
@@ -99,19 +137,23 @@ typedef struct VmdkExtent {
bool compressed;
bool has_marker;
bool has_zero_grain;
+bool sesparse;
+uint64_t sesparse_l2_tables_offset;
+uint64_t sesparse_clusters_offset;
+int32_t entry_size;
int version;
int64_t sectors;
int64_t end_sector;
int64_t flat_start_offset;
int64_t l1_table_offset;
int64_t l1_backup_table_offset;
-uint32_t *l1_table;
+void *l1_table;
uint32_t *l1_backup_table;
unsigned int l1_size;
uint32_t l1_entry_sectors;
unsigned int l2_size;
-uint32_t *l2_cache;
+void *l2_cache;
uint32_t l2_cache_offsets[L2_CACHE_SIZE];
uint32_t l2_cache_counts[L2_CACHE_SIZE];
@@ -435,6 +477,11 @@ static int vmdk_add_extent(BlockDriverState *bs,
*minimal L2 table size: 512 entries
*8 TB is still more than the maximal value supported for
*VMDK3 & VMDK4 which is 2TB.
+ * 64TB - for "ESXi seSparse Extent"
+ *minimal cluster size: 512B (default is 4KB)
+ *L2 table size: 4096 entries (const).
+ *64TB is more than the maximal value supported for
+ *seSparse VMDKs (which is slightly less than 64TB)
*/
error_setg(errp, "L1 size too big");
return -EFBIG;
@@ -460,6 +507,7 @@ static int vmdk_add_extent(BlockDriverState *bs,
extent->l2_size = l2_size;
extent->cluster_sectors = flat ? sectors : cluster_sectors;
extent->next_cluster_sector = ROUND_UP(nb_sectors, cluster_sectors);
+extent->entry_size = sizeof(uint32_t);
if (s->num_extents > 1) {
extent->end_sector = (*(extent - 1)).end_sector + extent->sectors;
@@ -481,7 +529,7 @@ static int vmdk_init_tables(BlockDriverState *bs,
VmdkExtent *extent,
int i;
/* read the L1 table */
-l1_size = extent->l1_size * sizeof(uint32_t);
+l1_size = extent->l1_size * extent->entry_size;
extent->l1_table = g_try_malloc(l1_size);
if (l1_size && extent->l1_table == NULL) {
return -ENOMEM;
@@ -499,10 +547,16 @@ static int vmdk_init_tables(BlockDriverState *bs,
VmdkExtent *extent,
goto fail_l1;
}
for (i = 0; i < extent->l1_size; i++) {
-le32_to_cpus(&extent->l1_table[i]);
+if (extent->entry_size == sizeof(uint64_t)) {
+le64_to_cpus((uint64_t *)extent->l1_table + i);
+} else {
+assert(extent->entry_size == sizeof(uint32_t));
+le32_to_cpus((uint32_t *)extent-&g
Re: [Qemu-devel] [PATCH v2 3/3] vmdk: Add read-only support for seSparse snapshots
> On 19 Jun 2019, at 20:12, Max Reitz wrote:
>
> On 05.06.19 14:17, Sam Eiderman wrote:
>> Until ESXi 6.5 VMware used the vmfsSparse format for snapshots (VMDK3 in
>> QEMU).
>>
>> This format was lacking in the following:
>>
>>* Grain directory (L1) and grain table (L2) entries were 32-bit,
>> allowing access to only 2TB (slightly less) of data.
>>* The grain size (default) was 512 bytes - leading to data
>> fragmentation and many grain tables.
>>* For space reclamation purposes, it was necessary to find all the
>> grains which are not pointed to by any grain table - so a reverse
>> mapping of "offset of grain in vmdk" to "grain table" must be
>> constructed - which takes large amounts of CPU/RAM.
>>
>> The format specification can be found in VMware's documentation:
>> https://www.vmware.com/support/developer/vddk/vmdk_50_technote.pdf
>>
>> In ESXi 6.5, to support snapshot files larger than 2TB, a new format was
>> introduced: SESparse (Space Efficient).
>>
>> This format fixes the above issues:
>>
>>* All entries are now 64-bit.
>>* The grain size (default) is 4KB.
>>* Grain directory and grain tables are now located at the beginning
>> of the file.
>> + seSparse format reserves space for all grain tables.
>> + Grain tables can be addressed using an index.
>> + Grains are located in the end of the file and can also be
>>addressed with an index.
>> - seSparse vmdks of large disks (64TB) have huge preallocated
>>headers - mainly due to L2 tables, even for empty snapshots.
>>* The header contains a reverse mapping ("backmap") of "offset of
>> grain in vmdk" to "grain table" and a bitmap ("free bitmap") which
>> specifies for each grain - whether it is allocated or not.
>> Using these data structures we can implement space reclamation
>> efficiently.
>>* Due to the fact that the header now maintains two mappings:
>>* The regular one (grain directory & grain tables)
>>* A reverse one (backmap and free bitmap)
>> These data structures can lose consistency upon crash and result
>> in a corrupted VMDK.
>> Therefore, a journal is also added to the VMDK and is replayed
>> when the VMware reopens the file after a crash.
>>
>> Since ESXi 6.7 - SESparse is the only snapshot format available.
>>
>> Unfortunately, VMware does not provide documentation regarding the new
>> seSparse format.
>>
>> This commit is based on black-box research of the seSparse format.
>> Various in-guest block operations and their effect on the snapshot file
>> were tested.
>>
>> The only VMware provided source of information (regarding the underlying
>> implementation) was a log file on the ESXi:
>>
>>/var/log/hostd.log
>>
>> Whenever an seSparse snapshot is created - the log is being populated
>> with seSparse records.
>>
>> Relevant log records are of the form:
>>
>> [...] Const Header:
>> [...] constMagic = 0xcafebabe
>> [...] version= 2.1
>> [...] capacity = 204800
>> [...] grainSize = 8
>> [...] grainTableSize = 64
>> [...] flags = 0
>> [...] Extents:
>> [...] Header : <1 : 1>
>> [...] JournalHdr : <2 : 2>
>> [...] Journal: <2048 : 2048>
>> [...] GrainDirectory : <4096 : 2048>
>> [...] GrainTables: <6144 : 2048>
>> [...] FreeBitmap : <8192 : 2048>
>> [...] BackMap: <10240 : 2048>
>> [...] Grain : <12288 : 204800>
>> [...] Volatile Header:
>> [...] volatileMagic = 0xcafecafe
>> [...] FreeGTNumber = 0
>> [...] nextTxnSeqNumber = 0
>> [...] replayJournal = 0
>>
>> The sizes that are seen in the log file are in sectors.
>> Extents are of the following format:
>>
>> This commit is a strict implementation which enforces:
>>* magics
>>* version number 2.1
>>* grain size of 8 sectors (4KB)
>>* grain table size of 64 sectors
>>* zero flags
>>* extent locations
>>
>> Additionally, this commit proivdes only a subset of the functionality
>> offered by seSparse's format:
>>* Read-only
>>* No journal replay
>>* No space reclamation
>>* No unmap support
>>
>> Hence, jo
Re: [Qemu-devel] [SeaBIOS] [PATCH v3 4/4] geometry: Apply LCHS values for boot devices
> On 20 Jun 2019, at 8:42, Gerd Hoffmann wrote:
>
>> +static int
>> +overriden_lchs_supplied(struct drive_s *drive)
>> +{
>> +return drive->lchs.cylinder || drive->lchs.head || drive->lchs.sector;
>> +}
>
>> +case TRANSLATION_MACHINE:
>
> Hmm, why this name? Doesn't look intuitive to me.
TRANSLATION_HOST?
>
>> +desc = "overriden";
>
> I'd name that "host-supplied" or "fw-cfg”.
“host-supplied”?
>
>> +cylinders = drive->lchs.cylinder;
>> +heads = drive->lchs.head;
>> +if (heads > 255)
>> +heads = 255;
>
> I suggest to move these sanity checks to overriden_lchs_supplied(), then
> ignore the override altogether when heads or sectors is out of range
> instead of trying to fixup things.
Sounds reasonable.
I’ll rename to host_lchs_supplied()?
WDYT?
>
> The other patches look all sane to me.
>
> cheers,
> Gerd
>
[Qemu-devel] [PATCH v3 2/3] vmdk: Reduce the max bound for L1 table size
512M of L1 entries is a very loose bound, only 32M are required to store
the maximal supported VMDK file size of 2TB.
Fixed qemu-iotest 59# - now failure occures before on impossible L1
table size.
Reviewed-by: Karl Heubaum
Reviewed-by: Eyal Moscovici
Reviewed-by: Liran Alon
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
block/vmdk.c | 13 +++--
tests/qemu-iotests/059.out | 2 +-
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 0f2e453bf5..931eb2759c 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -425,15 +425,16 @@ static int vmdk_add_extent(BlockDriverState *bs,
error_setg(errp, "Invalid granularity, image may be corrupt");
return -EFBIG;
}
-if (l1_size > 512 * 1024 * 1024) {
+if (l1_size > 32 * 1024 * 1024) {
/*
* Although with big capacity and small l1_entry_sectors, we can get a
* big l1_size, we don't want unbounded value to allocate the table.
- * Limit it to 512M, which is:
- * 16PB - for default "Hosted Sparse Extent" (VMDK4)
- *cluster size: 64KB, L2 table size: 512 entries
- * 1PB - for default "ESXi Host Sparse Extent" (VMDK3/vmfsSparse)
- *cluster size: 512B, L2 table size: 4096 entries
+ * Limit it to 32M, which is enough to store:
+ * 8TB - for both VMDK3 & VMDK4 with
+ *minimal cluster size: 512B
+ *minimal L2 table size: 512 entries
+ *8 TB is still more than the maximal value supported for
+ *VMDK3 & VMDK4 which is 2TB.
*/
error_setg(errp, "L1 size too big");
return -EFBIG;
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index f51394ae8e..4fab42a28c 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -2358,5 +2358,5 @@ Offset Length Mapped to File
0x14000 0x1 0x5 TEST_DIR/t-s003.vmdk
=== Testing afl image with a very large capacity ===
-qemu-img: Can't get image size 'TEST_DIR/afl9.IMGFMT': File too large
+qemu-img: Could not open 'TEST_DIR/afl9.IMGFMT': L1 size too big
*** done
--
2.13.3
[Qemu-devel] [PATCH v3 1/3] vmdk: Fix comment regarding max l1_size coverage
Commit b0651b8c246d ("vmdk: Move l1_size check into vmdk_add_extent")
extended the l1_size check from VMDK4 to VMDK3 but did not update the
default coverage in the moved comment.
The previous vmdk4 calculation:
(512 * 1024 * 1024) * 512(l2 entries) * 65536(grain) = 16PB
The added vmdk3 calculation:
(512 * 1024 * 1024) * 4096(l2 entries) * 512(grain) = 1PB
Adding the calculation of vmdk3 to the comment.
In any case, VMware does not offer virtual disks more than 2TB for
vmdk4/vmdk3 or 64TB for the new undocumented seSparse format which is
not implemented yet in qemu.
Reviewed-by: Karl Heubaum
Reviewed-by: Eyal Moscovici
Reviewed-by: Liran Alon
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
block/vmdk.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 51067c774f..0f2e453bf5 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -426,10 +426,15 @@ static int vmdk_add_extent(BlockDriverState *bs,
return -EFBIG;
}
if (l1_size > 512 * 1024 * 1024) {
-/* Although with big capacity and small l1_entry_sectors, we can get a
+/*
+ * Although with big capacity and small l1_entry_sectors, we can get a
* big l1_size, we don't want unbounded value to allocate the table.
- * Limit it to 512M, which is 16PB for default cluster and L2 table
- * size */
+ * Limit it to 512M, which is:
+ * 16PB - for default "Hosted Sparse Extent" (VMDK4)
+ *cluster size: 64KB, L2 table size: 512 entries
+ * 1PB - for default "ESXi Host Sparse Extent" (VMDK3/vmfsSparse)
+ *cluster size: 512B, L2 table size: 4096 entries
+ */
error_setg(errp, "L1 size too big");
return -EFBIG;
}
--
2.13.3
[Qemu-devel] [QEMU] [PATCH v4 0/8] Add Qemu to SeaBIOS LCHS interface
v1: Non-standard logical geometries break under QEMU. A virtual disk which contains an operating system which depends on logical geometries (consistent values being reported from BIOS INT13 AH=08) will most likely break under QEMU/SeaBIOS if it has non-standard logical geometries - for example 56 SPT (sectors per track). No matter what QEMU will guess - SeaBIOS, for large enough disks - will use LBA translation, which will report 63 SPT instead. In addition we can not enforce SeaBIOS to rely on phyiscal geometries at all. A virtio-blk-pci virtual disk with 255 phyiscal heads can not report more than 16 physical heads when moved to an IDE controller, the ATA spec allows a maximum of 16 heads - this is an artifact of virtualization. By supplying the logical geometies directly we are able to support such "exotic" disks. We will use fw_cfg to do just that. v2: Fix missing parenthesis check in "hd-geo-test: Add tests for lchs override" v3: * Rename fw_cfg key to "bios-geometry". * Remove "extendible" interface. * Add cpu_to_le32 fix as Laszlo suggested or big endian hosts * Fix last qtest commit - automatic docker tester for some reason does not have qemu-img set v4: * Change fw_cfg interface from mixed textual/binary to textual only Sam Eiderman (8): block: Refactor macros - fix tabbing block: Support providing LCHS from user bootdevice: Add interface to gather LCHS scsi: Propagate unrealize() callback to scsi-hd bootdevice: Gather LCHS from all relevant devices bootdevice: Refactor get_boot_devices_list bootdevice: FW_CFG interface for LCHS values hd-geo-test: Add tests for lchs override bootdevice.c | 148 +--- hw/block/virtio-blk.c| 6 + hw/ide/qdev.c| 7 +- hw/nvram/fw_cfg.c| 14 +- hw/scsi/scsi-bus.c | 15 ++ hw/scsi/scsi-disk.c | 14 ++ include/hw/block/block.h | 22 +- include/hw/scsi/scsi.h | 1 + include/sysemu/sysemu.h | 4 + tests/Makefile.include | 2 +- tests/hd-geo-test.c | 581 +++ 11 files changed, 773 insertions(+), 41 deletions(-) -- 2.13.3
[Qemu-devel] [QEMU] [PATCH v4 7/8] bootdevice: FW_CFG interface for LCHS values
Using fw_cfg, supply logical CHS values directly from QEMU to the BIOS.
Non-standard logical geometries break under QEMU.
A virtual disk which contains an operating system which depends on
logical geometries (consistent values being reported from BIOS INT13
AH=08) will most likely break under QEMU/SeaBIOS if it has non-standard
logical geometries - for example 56 SPT (sectors per track).
No matter what QEMU will report - SeaBIOS, for large enough disks - will
use LBA translation, which will report 63 SPT instead.
In addition we cannot force SeaBIOS to rely on physical geometries at
all. A virtio-blk-pci virtual disk with 255 phyiscal heads cannot
report more than 16 physical heads when moved to an IDE controller,
since the ATA spec allows a maximum of 16 heads - this is an artifact of
virtualization.
By supplying the logical geometries directly we are able to support such
"exotic" disks.
We serialize this information in a similar way to the "bootorder"
interface.
The new fw_cfg entry is "bios-geometry".
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
bootdevice.c| 32
hw/nvram/fw_cfg.c | 14 +++---
include/sysemu/sysemu.h | 1 +
3 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/bootdevice.c b/bootdevice.c
index 2b12fb85a4..b034ad7bdc 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -405,3 +405,35 @@ void del_boot_device_lchs(DeviceState *dev, const char
*suffix)
}
}
}
+
+/* Serialized as: (device name\0 + lchs struct) x devices */
+char *get_boot_devices_lchs_list(size_t *size)
+{
+FWLCHSEntry *i;
+size_t total = 0;
+char *list = NULL;
+
+QTAILQ_FOREACH(i, &fw_lchs, link) {
+char *bootpath;
+char *chs_string;
+size_t len;
+
+bootpath = get_boot_device_path(i->dev, false, i->suffix);
+chs_string = g_strdup_printf("%s %" PRIu32 " %" PRIu32 " %" PRIu32,
+ bootpath, i->lcyls, i->lheads, i->lsecs);
+
+if (total) {
+list[total - 1] = '\n';
+}
+len = strlen(chs_string) + 1;
+list = g_realloc(list, total + len);
+memcpy(&list[total], chs_string, len);
+total += len;
+g_free(chs_string);
+g_free(bootpath);
+}
+
+*size = total;
+
+return list;
+}
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 9f7b7789bc..c1230fe11c 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -916,13 +916,21 @@ void *fw_cfg_modify_file(FWCfgState *s, const char
*filename,
static void fw_cfg_machine_reset(void *opaque)
{
+MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
+FWCfgState *s = opaque;
void *ptr;
size_t len;
-FWCfgState *s = opaque;
-char *bootindex = get_boot_devices_list(&len);
+char *buf;
-ptr = fw_cfg_modify_file(s, "bootorder", (uint8_t *)bootindex, len);
+buf = get_boot_devices_list(&len);
+ptr = fw_cfg_modify_file(s, "bootorder", (uint8_t *)buf, len);
g_free(ptr);
+
+if (!mc->legacy_fw_cfg_order) {
+buf = get_boot_devices_lchs_list(&len);
+ptr = fw_cfg_modify_file(s, "bios-geometry", (uint8_t *)buf, len);
+g_free(ptr);
+}
}
static void fw_cfg_machine_ready(struct Notifier *n, void *data)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 173dfbb539..1ca8799588 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -174,6 +174,7 @@ void validate_bootdevices(const char *devices, Error
**errp);
void add_boot_device_lchs(DeviceState *dev, const char *suffix,
uint32_t lcyls, uint32_t lheads, uint32_t lsecs);
void del_boot_device_lchs(DeviceState *dev, const char *suffix);
+char *get_boot_devices_lchs_list(size_t *size);
/* handler to set the boot_device order for a specific type of MachineClass */
typedef void QEMUBootSetHandler(void *opaque, const char *boot_order,
--
2.13.3
[Qemu-devel] [QEMU] [PATCH v4 4/8] scsi: Propagate unrealize() callback to scsi-hd
We will need to add LCHS removal logic to scsi-hd's unrealize() in the
next commit.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
hw/scsi/scsi-bus.c | 15 +++
include/hw/scsi/scsi.h | 1 +
2 files changed, 16 insertions(+)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index c480553083..f6fe497a1a 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -55,6 +55,14 @@ static void scsi_device_realize(SCSIDevice *s, Error **errp)
}
}
+static void scsi_device_unrealize(SCSIDevice *s, Error **errp)
+{
+SCSIDeviceClass *sc = SCSI_DEVICE_GET_CLASS(s);
+if (sc->unrealize) {
+sc->unrealize(s, errp);
+}
+}
+
int scsi_bus_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
void *hba_private)
{
@@ -213,11 +221,18 @@ static void scsi_qdev_realize(DeviceState *qdev, Error
**errp)
static void scsi_qdev_unrealize(DeviceState *qdev, Error **errp)
{
SCSIDevice *dev = SCSI_DEVICE(qdev);
+Error *local_err = NULL;
if (dev->vmsentry) {
qemu_del_vm_change_state_handler(dev->vmsentry);
}
+scsi_device_unrealize(dev, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
scsi_device_purge_requests(dev, SENSE_CODE(NO_SENSE));
blockdev_mark_auto_del(dev->conf.blk);
}
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 426566a5c6..8cf71f910d 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -59,6 +59,7 @@ struct SCSIRequest {
typedef struct SCSIDeviceClass {
DeviceClass parent_class;
void (*realize)(SCSIDevice *dev, Error **errp);
+void (*unrealize)(SCSIDevice *dev, Error **errp);
int (*parse_cdb)(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
void *hba_private);
SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun,
--
2.13.3
[Qemu-devel] [QEMU] [PATCH v4 5/8] bootdevice: Gather LCHS from all relevant devices
Relevant devices are:
* ide-hd (and ide-cd, ide-drive)
* scsi-hd (and scsi-cd, scsi-disk, scsi-block)
* virtio-blk-pci
We do not call del_boot_device_lchs() for ide-* since we don't need to -
IDE block devices do not support unplugging.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
hw/block/virtio-blk.c | 6 ++
hw/ide/qdev.c | 5 +
hw/scsi/scsi-disk.c | 14 ++
3 files changed, 25 insertions(+)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 06e57a4d39..787bbd768a 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1182,6 +1182,11 @@ static void virtio_blk_device_realize(DeviceState *dev,
Error **errp)
blk_set_guest_block_size(s->blk, s->conf.conf.logical_block_size);
blk_iostatus_enable(s->blk);
+
+add_boot_device_lchs(dev, "/disk@0,0",
+ (&conf->conf)->lcyls,
+ (&conf->conf)->lheads,
+ (&conf->conf)->lsecs);
}
static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp)
@@ -1189,6 +1194,7 @@ static void virtio_blk_device_unrealize(DeviceState *dev,
Error **errp)
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
VirtIOBlock *s = VIRTIO_BLK(dev);
+del_boot_device_lchs(dev, "/disk@0,0");
virtio_blk_data_plane_destroy(s->dataplane);
s->dataplane = NULL;
qemu_del_vm_change_state_handler(s->change);
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 9cae3205df..07f429d5e3 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -215,6 +215,11 @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind
kind, Error **errp)
add_boot_device_path(dev->conf.bootindex, &dev->qdev,
dev->unit ? "/disk@1" : "/disk@0");
+
+add_boot_device_lchs(&dev->qdev, dev->unit ? "/disk@1" : "/disk@0",
+ (&dev->conf)->lcyls,
+ (&dev->conf)->lheads,
+ (&dev->conf)->lsecs);
}
static void ide_dev_get_bootindex(Object *obj, Visitor *v, const char *name,
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 7b89ac798b..3451aefdea 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2390,6 +2390,16 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
blk_set_guest_block_size(s->qdev.conf.blk, s->qdev.blocksize);
blk_iostatus_enable(s->qdev.conf.blk);
+
+add_boot_device_lchs(&dev->qdev, NULL,
+ (&dev->conf)->lcyls,
+ (&dev->conf)->lheads,
+ (&dev->conf)->lsecs);
+}
+
+static void scsi_unrealize(SCSIDevice *dev, Error **errp)
+{
+del_boot_device_lchs(&dev->qdev, NULL);
}
static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
@@ -2988,6 +2998,7 @@ static void scsi_hd_class_initfn(ObjectClass *klass, void
*data)
SCSIDeviceClass *sc = SCSI_DEVICE_CLASS(klass);
sc->realize = scsi_hd_realize;
+sc->unrealize= scsi_unrealize;
sc->alloc_req= scsi_new_request;
sc->unit_attention_reported = scsi_disk_unit_attention_reported;
dc->desc = "virtual SCSI disk";
@@ -3019,6 +3030,7 @@ static void scsi_cd_class_initfn(ObjectClass *klass, void
*data)
SCSIDeviceClass *sc = SCSI_DEVICE_CLASS(klass);
sc->realize = scsi_cd_realize;
+sc->unrealize= scsi_unrealize;
sc->alloc_req= scsi_new_request;
sc->unit_attention_reported = scsi_disk_unit_attention_reported;
dc->desc = "virtual SCSI CD-ROM";
@@ -3054,6 +3066,7 @@ static void scsi_block_class_initfn(ObjectClass *klass,
void *data)
SCSIDiskClass *sdc = SCSI_DISK_BASE_CLASS(klass);
sc->realize = scsi_block_realize;
+sc->unrealize= scsi_unrealize;
sc->alloc_req= scsi_block_new_request;
sc->parse_cdb= scsi_block_parse_cdb;
sdc->dma_readv = scsi_block_dma_readv;
@@ -3095,6 +3108,7 @@ static void scsi_disk_class_initfn(ObjectClass *klass,
void *data)
SCSIDeviceClass *sc = SCSI_DEVICE_CLASS(klass);
sc->realize = scsi_disk_realize;
+sc->unrealize= scsi_unrealize;
sc->alloc_req= scsi_new_request;
sc->unit_attention_reported = scsi_disk_unit_attention_reported;
dc->fw_name = "disk";
--
2.13.3
Re: [Qemu-devel] [PATCH v2 0/3] vmdk: Add read-only support for the new seSparse format
Gentle ping > On 5 Jun 2019, at 15:17, Sam Eiderman wrote: > > v1: > > VMware introduced a new snapshot format in VMFS6 - seSparse (Space > Efficient Sparse) which is the default format available in ESXi 6.7. > Add read-only support for the new snapshot format. > > v2: > > Fixed after Max's review: > > * Removed strict sesparse checks > * Reduced maximal L1 table size > * Added non-write mode check in vmdk_open() on sesparse > > Sam Eiderman (3): > vmdk: Fix comment regarding max l1_size coverage > vmdk: Reduce the max bound for L1 table size > vmdk: Add read-only support for seSparse snapshots > > block/vmdk.c | 371 ++--- > tests/qemu-iotests/059.out | 2 +- > 2 files changed, 352 insertions(+), 21 deletions(-) > > -- > 2.13.3 >
[Qemu-devel] [QEMU] [PATCH v4 3/8] bootdevice: Add interface to gather LCHS
Add an interface to provide direct logical CHS values for boot devices.
We will use this interface in the next commits.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
bootdevice.c| 55 +
include/sysemu/sysemu.h | 3 +++
2 files changed, 58 insertions(+)
diff --git a/bootdevice.c b/bootdevice.c
index 1d225202f9..bc5e1c2de4 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -343,3 +343,58 @@ void device_add_bootindex_property(Object *obj, int32_t
*bootindex,
/* initialize devices' bootindex property to -1 */
object_property_set_int(obj, -1, name, NULL);
}
+
+typedef struct FWLCHSEntry FWLCHSEntry;
+
+struct FWLCHSEntry {
+QTAILQ_ENTRY(FWLCHSEntry) link;
+DeviceState *dev;
+char *suffix;
+uint32_t lcyls;
+uint32_t lheads;
+uint32_t lsecs;
+};
+
+static QTAILQ_HEAD(, FWLCHSEntry) fw_lchs =
+QTAILQ_HEAD_INITIALIZER(fw_lchs);
+
+void add_boot_device_lchs(DeviceState *dev, const char *suffix,
+ uint32_t lcyls, uint32_t lheads, uint32_t lsecs)
+{
+FWLCHSEntry *node;
+
+if (!lcyls && !lheads && !lsecs) {
+return;
+}
+
+assert(dev != NULL || suffix != NULL);
+
+node = g_malloc0(sizeof(FWLCHSEntry));
+node->suffix = g_strdup(suffix);
+node->dev = dev;
+node->lcyls = lcyls;
+node->lheads = lheads;
+node->lsecs = lsecs;
+
+QTAILQ_INSERT_TAIL(&fw_lchs, node, link);
+}
+
+void del_boot_device_lchs(DeviceState *dev, const char *suffix)
+{
+FWLCHSEntry *i;
+
+if (dev == NULL) {
+return;
+}
+
+QTAILQ_FOREACH(i, &fw_lchs, link) {
+if ((!suffix || !g_strcmp0(i->suffix, suffix)) &&
+ i->dev == dev) {
+QTAILQ_REMOVE(&fw_lchs, i, link);
+g_free(i->suffix);
+g_free(i);
+
+break;
+}
+}
+}
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 61579ae71e..173dfbb539 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -171,6 +171,9 @@ void device_add_bootindex_property(Object *obj, int32_t
*bootindex,
DeviceState *dev, Error **errp);
void restore_boot_order(void *opaque);
void validate_bootdevices(const char *devices, Error **errp);
+void add_boot_device_lchs(DeviceState *dev, const char *suffix,
+ uint32_t lcyls, uint32_t lheads, uint32_t lsecs);
+void del_boot_device_lchs(DeviceState *dev, const char *suffix);
/* handler to set the boot_device order for a specific type of MachineClass */
typedef void QEMUBootSetHandler(void *opaque, const char *boot_order,
--
2.13.3
[Qemu-devel] [QEMU] [PATCH v4 8/8] hd-geo-test: Add tests for lchs override
Add QTest tests to check the logical geometry override option.
The tests in hd-geo-test are out of date - they only test IDE and do not
test interesting MBRs.
I added a few helper functions which will make adding more tests easier.
QTest's fw_cfg helper functions support only legacy fw_cfg, so I had to
read the new fw_cfg layout on my own.
Creating qcow2 disks with specific size and MBR layout is currently
unused - we only use a default empty MBR.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
tests/Makefile.include | 2 +-
tests/hd-geo-test.c| 581 +
2 files changed, 582 insertions(+), 1 deletion(-)
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 46a36c2c95..55ea165ed4 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -765,7 +765,7 @@ tests/ide-test$(EXESUF): tests/ide-test.o $(libqos-pc-obj-y)
tests/ahci-test$(EXESUF): tests/ahci-test.o $(libqos-pc-obj-y)
qemu-img$(EXESUF)
tests/ipmi-kcs-test$(EXESUF): tests/ipmi-kcs-test.o
tests/ipmi-bt-test$(EXESUF): tests/ipmi-bt-test.o
-tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o
+tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o $(libqos-obj-y)
tests/boot-order-test$(EXESUF): tests/boot-order-test.o $(libqos-obj-y)
tests/boot-serial-test$(EXESUF): tests/boot-serial-test.o $(libqos-obj-y)
tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
index 62eb624726..597678e118 100644
--- a/tests/hd-geo-test.c
+++ b/tests/hd-geo-test.c
@@ -17,7 +17,12 @@
#include "qemu/osdep.h"
#include "qemu-common.h"
+#include "qemu/bswap.h"
+#include "qapi/qmp/qlist.h"
#include "libqtest.h"
+#include "libqos/fw_cfg.h"
+#include "libqos/libqos.h"
+#include "standard-headers/linux/qemu_fw_cfg.h"
#define ARGV_SIZE 256
@@ -388,6 +393,567 @@ static void test_ide_drive_cd_0(void)
qtest_quit(qts);
}
+typedef struct {
+bool active;
+uint32_t head;
+uint32_t sector;
+uint32_t cyl;
+uint32_t end_head;
+uint32_t end_sector;
+uint32_t end_cyl;
+uint32_t start_sect;
+uint32_t nr_sects;
+} MBRpartitions[4];
+
+static MBRpartitions empty_mbr = { {false, 0, 0, 0, 0, 0, 0, 0, 0},
+ {false, 0, 0, 0, 0, 0, 0, 0, 0},
+ {false, 0, 0, 0, 0, 0, 0, 0, 0},
+ {false, 0, 0, 0, 0, 0, 0, 0, 0} };
+
+static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
+{
+const char *template = "/tmp/qtest.XX";
+char *raw_path = strdup(template);
+char *qcow2_path = strdup(template);
+char cmd[100 + 2 * PATH_MAX];
+uint8_t buf[512];
+int i, ret, fd, offset;
+uint64_t qcow2_size = sectors * 512;
+uint8_t status, parttype, head, sector, cyl;
+char *qemu_img_path;
+char *qemu_img_abs_path;
+
+offset = 0xbe;
+
+for (i = 0; i < 4; i++) {
+status = mbr[i].active ? 0x80 : 0x00;
+g_assert(mbr[i].head < 256);
+g_assert(mbr[i].sector < 64);
+g_assert(mbr[i].cyl < 1024);
+head = mbr[i].head;
+sector = mbr[i].sector + ((mbr[i].cyl & 0x300) >> 2);
+cyl = mbr[i].cyl & 0xff;
+
+buf[offset + 0x0] = status;
+buf[offset + 0x1] = head;
+buf[offset + 0x2] = sector;
+buf[offset + 0x3] = cyl;
+
+parttype = 0;
+g_assert(mbr[i].end_head < 256);
+g_assert(mbr[i].end_sector < 64);
+g_assert(mbr[i].end_cyl < 1024);
+head = mbr[i].end_head;
+sector = mbr[i].end_sector + ((mbr[i].end_cyl & 0x300) >> 2);
+cyl = mbr[i].end_cyl & 0xff;
+
+buf[offset + 0x4] = parttype;
+buf[offset + 0x5] = head;
+buf[offset + 0x6] = sector;
+buf[offset + 0x7] = cyl;
+
+(*(uint32_t *)&buf[offset + 0x8]) = cpu_to_le32(mbr[i].start_sect);
+(*(uint32_t *)&buf[offset + 0xc]) = cpu_to_le32(mbr[i].nr_sects);
+
+offset += 0x10;
+}
+
+fd = mkstemp(raw_path);
+g_assert(fd);
+close(fd);
+
+fd = open(raw_path, O_WRONLY);
+g_assert(fd >= 0);
+ret = write(fd, buf, sizeof(buf));
+g_assert(ret == sizeof(buf));
+close(fd);
+
+fd = mkstemp(qcow2_path);
+g_assert(fd);
+close(fd);
+
+qemu_img_path = getenv("QTEST_QEMU_IMG");
+g_assert(qemu_img_path);
+qemu_img_abs_path = realpath(qemu_img_path, NULL);
+g_assert(qemu_img_abs_path);
+
+ret = snprintf(cmd, sizeof(cmd),
+ "%s convert -f raw -O qcow2 %s %s > /dev/null",
+ qemu_img_abs_path,
+ raw_path, qcow2_path);
+g_assert((0 < ret) && (ret <= sizeof(cmd)));
+ret = system(cmd);
+g_assert(ret == 0);
+
+ret = s
[Qemu-devel] [QEMU] [PATCH v4 1/8] block: Refactor macros - fix tabbing
Fixing tabbing in block related macros.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
hw/ide/qdev.c| 2 +-
include/hw/block/block.h | 16
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 360cd20bd8..9cae3205df 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -285,7 +285,7 @@ static void ide_drive_realize(IDEDevice *dev, Error **errp)
DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf),\
DEFINE_BLOCK_ERROR_PROPERTIES(IDEDrive, dev.conf), \
DEFINE_PROP_STRING("ver", IDEDrive, dev.version), \
-DEFINE_PROP_UINT64("wwn", IDEDrive, dev.wwn, 0),\
+DEFINE_PROP_UINT64("wwn", IDEDrive, dev.wwn, 0), \
DEFINE_PROP_STRING("serial", IDEDrive, dev.serial),\
DEFINE_PROP_STRING("model", IDEDrive, dev.model)
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index 607539057a..fd55a30bca 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -50,21 +50,21 @@ static inline unsigned int get_physical_block_exp(BlockConf
*conf)
_conf.logical_block_size),\
DEFINE_PROP_BLOCKSIZE("physical_block_size", _state,\
_conf.physical_block_size), \
-DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0), \
+DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0),\
DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0),\
-DEFINE_PROP_UINT32("discard_granularity", _state, \
- _conf.discard_granularity, -1), \
-DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, \
-ON_OFF_AUTO_AUTO), \
+DEFINE_PROP_UINT32("discard_granularity", _state, \
+ _conf.discard_granularity, -1), \
+DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, \
+ON_OFF_AUTO_AUTO), \
DEFINE_PROP_BOOL("share-rw", _state, _conf.share_rw, false)
#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \
DEFINE_PROP_DRIVE("drive", _state, _conf.blk), \
DEFINE_BLOCK_PROPERTIES_BASE(_state, _conf)
-#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \
-DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \
-DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0), \
+#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \
+DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \
+DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0),\
DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0)
#define DEFINE_BLOCK_ERROR_PROPERTIES(_state, _conf)\
--
2.13.3
[Qemu-devel] [QEMU] [PATCH v4 2/8] block: Support providing LCHS from user
Add logical geometry variables to BlockConf.
A user can now supply "lcyls", "lheads" & "lsecs" for any HD device
that supports CHS ("cyls", "heads", "secs").
These devices include:
* ide-hd
* scsi-hd
* virtio-blk-pci
In future commits we will use the provided LCHS and pass it to the BIOS
through fw_cfg to be supplied using INT13 routines.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
include/hw/block/block.h | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index fd55a30bca..d7246f3862 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -26,6 +26,7 @@ typedef struct BlockConf {
uint32_t discard_granularity;
/* geometry, not all devices use this */
uint32_t cyls, heads, secs;
+uint32_t lcyls, lheads, lsecs;
OnOffAuto wce;
bool share_rw;
BlockdevOnError rerror;
@@ -65,7 +66,10 @@ static inline unsigned int get_physical_block_exp(BlockConf
*conf)
#define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \
DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \
DEFINE_PROP_UINT32("heads", _state, _conf.heads, 0),\
-DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0)
+DEFINE_PROP_UINT32("secs", _state, _conf.secs, 0), \
+DEFINE_PROP_UINT32("lcyls", _state, _conf.lcyls, 0),\
+DEFINE_PROP_UINT32("lheads", _state, _conf.lheads, 0), \
+DEFINE_PROP_UINT32("lsecs", _state, _conf.lsecs, 0)
#define DEFINE_BLOCK_ERROR_PROPERTIES(_state, _conf)\
DEFINE_PROP_BLOCKDEV_ON_ERROR("rerror", _state, _conf.rerror, \
--
2.13.3
[Qemu-devel] [QEMU] [PATCH v4 6/8] bootdevice: Refactor get_boot_devices_list
Move device name construction to a separate function.
We will reuse this function in the following commit to pass logical CHS
parameters through fw_cfg much like we currently pass bootindex.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
bootdevice.c | 61 +---
1 file changed, 34 insertions(+), 27 deletions(-)
diff --git a/bootdevice.c b/bootdevice.c
index bc5e1c2de4..2b12fb85a4 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -202,6 +202,39 @@ DeviceState *get_boot_device(uint32_t position)
return res;
}
+static char *get_boot_device_path(DeviceState *dev, bool ignore_suffixes,
+ char *suffix)
+{
+char *devpath = NULL, *s = NULL, *d, *bootpath;
+
+if (dev) {
+devpath = qdev_get_fw_dev_path(dev);
+assert(devpath);
+}
+
+if (!ignore_suffixes) {
+if (dev) {
+d = qdev_get_own_fw_dev_path_from_handler(dev->parent_bus, dev);
+if (d) {
+assert(!suffix);
+s = d;
+} else {
+s = g_strdup(suffix);
+}
+} else {
+s = g_strdup(suffix);
+}
+}
+
+bootpath = g_strdup_printf("%s%s",
+ devpath ? devpath : "",
+ s ? s : "");
+g_free(devpath);
+g_free(s);
+
+return bootpath;
+}
+
/*
* This function returns null terminated string that consist of new line
* separated device paths.
@@ -218,36 +251,10 @@ char *get_boot_devices_list(size_t *size)
bool ignore_suffixes = mc->ignore_boot_device_suffixes;
QTAILQ_FOREACH(i, &fw_boot_order, link) {
-char *devpath = NULL, *suffix = NULL;
char *bootpath;
-char *d;
size_t len;
-if (i->dev) {
-devpath = qdev_get_fw_dev_path(i->dev);
-assert(devpath);
-}
-
-if (!ignore_suffixes) {
-if (i->dev) {
-d = qdev_get_own_fw_dev_path_from_handler(i->dev->parent_bus,
- i->dev);
-if (d) {
-assert(!i->suffix);
-suffix = d;
-} else {
-suffix = g_strdup(i->suffix);
-}
-} else {
-suffix = g_strdup(i->suffix);
-}
-}
-
-bootpath = g_strdup_printf("%s%s",
- devpath ? devpath : "",
- suffix ? suffix : "");
-g_free(devpath);
-g_free(suffix);
+bootpath = get_boot_device_path(i->dev, ignore_suffixes, i->suffix);
if (total) {
list[total-1] = '\n';
--
2.13.3
[Qemu-devel] [SeaBIOS] [PATCH v3 4/4] geometry: Apply LCHS values for boot devices
Boot devices which use overriden LCHS values are:
* ata
* ahci
* scsi
* esp
* lsi
* megasas
* mpt
* pvscsi
* virtio
* virtio-blk
We use these values in get_translation() and setup_translation() by
introducing a new translation type: "TRANSLATION_MACHINE".
We treat this translation as TRANSLATION_NONE in fill_ata_edd(),
although this does not really matter since now the translation between
physical and logical geometry does not exist.
Reviewed-by: Karl Heubaum
Reviewed-by: Arbel Moshe
Signed-off-by: Sam Eiderman
---
src/block.c | 21 -
src/block.h | 1 +
src/hw/ahci.c| 1 +
src/hw/ata.c | 8
src/hw/esp-scsi.c| 2 ++
src/hw/lsi-scsi.c| 2 ++
src/hw/megasas.c | 1 +
src/hw/mpt-scsi.c| 2 ++
src/hw/pvscsi.c | 1 +
src/hw/virtio-blk.c | 2 ++
src/hw/virtio-scsi.c | 2 ++
11 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/src/block.c b/src/block.c
index f73ec18c..ca23a83a 100644
--- a/src/block.c
+++ b/src/block.c
@@ -69,9 +69,17 @@ int create_bounce_buf(void)
* Disk geometry translation
/
+static int
+overriden_lchs_supplied(struct drive_s *drive)
+{
+return drive->lchs.cylinder || drive->lchs.head || drive->lchs.sector;
+}
+
static u8
get_translation(struct drive_s *drive)
{
+if (overriden_lchs_supplied(drive))
+return TRANSLATION_MACHINE;
u8 type = drive->type;
if (CONFIG_QEMU && type == DTYPE_ATA) {
// Emulators pass in the translation info via nvram.
@@ -159,6 +167,16 @@ setup_translation(struct drive_s *drive)
break;
}
break;
+case TRANSLATION_MACHINE:
+desc = "overriden";
+cylinders = drive->lchs.cylinder;
+heads = drive->lchs.head;
+if (heads > 255)
+heads = 255;
+spt = drive->lchs.sector;
+if (spt > 63)
+spt = 63;
+break;
}
// clip to 1024 cylinders in lchs
if (cylinders > 1024)
@@ -423,7 +441,8 @@ fill_ata_edd(struct segoff_s edd, struct drive_s *drive_gf)
u16 options = 0;
if (GET_GLOBALFLAT(drive_gf->type) == DTYPE_ATA) {
u8 translation = GET_GLOBALFLAT(drive_gf->translation);
-if (translation != TRANSLATION_NONE) {
+if ((translation != TRANSLATION_NONE) &&
+(translation != TRANSLATION_MACHINE)) {
options |= 1<<3; // CHS translation
if (translation == TRANSLATION_LBA)
options |= 1<<9;
diff --git a/src/block.h b/src/block.h
index f64e8807..12f27eee 100644
--- a/src/block.h
+++ b/src/block.h
@@ -90,6 +90,7 @@ struct drive_s {
#define TRANSLATION_LBA 1
#define TRANSLATION_LARGE 2
#define TRANSLATION_RECHS 3
+#define TRANSLATION_MACHINE 4
#define EXTTYPE_FLOPPY 0
#define EXTTYPE_HD 1
diff --git a/src/hw/ahci.c b/src/hw/ahci.c
index 1746e7a1..97a072a1 100644
--- a/src/hw/ahci.c
+++ b/src/hw/ahci.c
@@ -593,6 +593,7 @@ static int ahci_port_setup(struct ahci_port_s *port)
, ata_extract_version(buffer));
port->prio = bootprio_find_ata_device(ctrl->pci_tmp, pnr, 0);
}
+boot_lchs_find_ata_device(ctrl->pci_tmp, pnr, 0, &(port->drive.lchs));
return 0;
}
diff --git a/src/hw/ata.c b/src/hw/ata.c
index b6e073cf..f788ce71 100644
--- a/src/hw/ata.c
+++ b/src/hw/ata.c
@@ -755,6 +755,10 @@ init_drive_atapi(struct atadrive_s *dummy, u16 *buffer)
int prio = bootprio_find_ata_device(adrive->chan_gf->pci_tmp,
adrive->chan_gf->chanid,
adrive->slave);
+boot_lchs_find_ata_device(adrive->chan_gf->pci_tmp,
+ adrive->chan_gf->chanid,
+ adrive->slave,
+ &(adrive->drive.lchs));
boot_add_cd(&adrive->drive, desc, prio);
}
@@ -805,6 +809,10 @@ init_drive_ata(struct atadrive_s *dummy, u16 *buffer)
int prio = bootprio_find_ata_device(adrive->chan_gf->pci_tmp,
adrive->chan_gf->chanid,
adrive->slave);
+boot_lchs_find_ata_device(adrive->chan_gf->pci_tmp,
+ adrive->chan_gf->chanid,
+ adrive->slave,
+ &(adrive->drive.lchs));
// Register with bcv system.
boot_add_hd(&adrive->drive, desc, prio);
diff --git a/src/hw/esp-scsi.c b/src/hw/esp-scsi.c
index ffd86d0f..cc25f227 100644
--- a/src/hw/esp-scsi.c
+++ b/src/hw/esp-scsi.c
@@ -181,6 +181,8 @@ esp_scsi_add_lun(u32 lun, struct drive_s *
[Qemu-devel] [SeaBIOS] [PATCH v3 0/4] Add Qemu to SeaBIOS LCHS interface
v1: Non-standard logical geometries break under QEMU. A virtual disk which contains an operating system which depends on logical geometries (consistent values being reported from BIOS INT13 AH=08) will most likely break under QEMU/SeaBIOS if it has non-standard logical geometries - for example 56 SPT (sectors per track). No matter what QEMU will guess - SeaBIOS, for large enough disks - will use LBA translation, which will report 63 SPT instead. In addition we can not enforce SeaBIOS to rely on phyiscal geometries at all. A virtio-blk-pci virtual disk with 255 phyiscal heads can not report more than 16 physical heads when moved to an IDE controller, the ATA spec allows a maximum of 16 heads - this is an artifact of virtualization. By supplying the logical geometies directly we are able to support such "exotic" disks. We will use fw_cfg to do just that. v2: Rename bootdevices fw_cfg key to bios-geoemtry v3: Change fw_cfg interface from mixed binary/textual to textual only Squash commit "config: Add toggle for bootdevice information" Sam Eiderman (4): geometry: Read LCHS from fw_cfg boot: Reorder functions in boot.c geometry: Add boot_lchs_find_*() utility functions geometry: Apply LCHS values for boot devices src/Kconfig | 7 ++ src/block.c | 21 - src/block.h | 1 + src/boot.c | 239 +-- src/hw/ahci.c| 1 + src/hw/ata.c | 8 ++ src/hw/esp-scsi.c| 2 + src/hw/lsi-scsi.c| 2 + src/hw/megasas.c | 1 + src/hw/mpt-scsi.c| 2 + src/hw/pvscsi.c | 1 + src/hw/virtio-blk.c | 2 + src/hw/virtio-scsi.c | 2 + src/util.h | 6 ++ 14 files changed, 249 insertions(+), 46 deletions(-) -- 2.13.3
