| Message ID | 1284645517-32743-1-git-send-email-mathias.krause@secunet.com |
|---|---|
| State | New |
| Headers | show |
Am 16.09.2010 15:58, schrieb Mathias Krause: > In contrast to the BIOS and Option ROMs the CMOS content cannot be > predefined by the user. Also the amount of useable CMOS ARM is pretty > limited, even though the amount of CMOS bytes emulated by qemu is > already twice as much as of the original MC146818. Nevertheless those > limitations are pretty annoying when testing different BIOS replacement > implementations like coreboot in combination with FILO that use CMOS > values above the basic RTC range for its own purpose to, e.g., control > the boot device order using a string containing the boot devices to > probe. > > This patch adds support to specify a file to read the initial CMOS > content from. It also increases the CMOS size to 256 bytes and > implements access to the extended memory range via I/O ports 0x72 and > 0x73. > > Use it like: `qemu -global mc146818rtc.file=cmos.bin ...' where cmos.bin > is a binary file, sized 256 bytes containing the CMOS RAM. > > Signed-off-by: Mathias Krause<mathias.krause@secunet.com> > --- > hw/mc146818rtc.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++----- > 1 files changed, 56 insertions(+), 6 deletions(-) > > [snip] The intention of this patch is ok. Loading CMOS with initial data is needed. I just want to add two questions / remarks how the implementation might be improved. Are there use cases where having a smaller CMOS size is better? For example, when I want to emulate a system with 128 byte CMOS? The size of CMOS could be derived from the size of the data or specified by an additional parameter. Using QEMU's block devices instead of a simple file would be more consistent with the rest of QEMU and allow reading the CMOS data not only from a file but also from an URL or other sources. Regards Stefan
On 09/16/2010 08:58 AM, Mathias Krause wrote: > In contrast to the BIOS and Option ROMs the CMOS content cannot be > predefined by the user. Also the amount of useable CMOS ARM is pretty > limited, even though the amount of CMOS bytes emulated by qemu is > already twice as much as of the original MC146818. Nevertheless those > limitations are pretty annoying when testing different BIOS replacement > implementations like coreboot in combination with FILO that use CMOS > values above the basic RTC range for its own purpose to, e.g., control > the boot device order using a string containing the boot devices to > probe. > > This patch adds support to specify a file to read the initial CMOS > content from. It also increases the CMOS size to 256 bytes and > implements access to the extended memory range via I/O ports 0x72 and > 0x73. > > Use it like: `qemu -global mc146818rtc.file=cmos.bin ...' where cmos.bin > is a binary file, sized 256 bytes containing the CMOS RAM. > > Signed-off-by: Mathias Krause<mathias.krause@secunet.com> > Instead of using FILE, I'd suggest using a BlockDriver to read and write the data. I think it would be very nice to add write support too so that writes to CMOS were persisted across boots. > --- > hw/mc146818rtc.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++----- > 1 files changed, 56 insertions(+), 6 deletions(-) > > diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c > index 2b91fa8..9f215e0 100644 > --- a/hw/mc146818rtc.c > +++ b/hw/mc146818rtc.c > @@ -78,12 +78,16 @@ > #define REG_C_PF 0x40 > #define REG_C_AF 0x20 > > +#define BASE_PORT 0x70 > +#define CMOS_SIZE 256 > + > typedef struct RTCState { > ISADevice dev; > - uint8_t cmos_data[128]; > + uint8_t cmos_data[CMOS_SIZE]; > uint8_t cmos_index; > struct tm current_tm; > int32_t base_year; > + char *file; > qemu_irq irq; > qemu_irq sqw_irq; > int it_shift; > @@ -206,7 +210,7 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data) > RTCState *s = opaque; > > if ((addr& 1) == 0) { > - s->cmos_index = data& 0x7f; > + s->cmos_index = data& (addr == BASE_PORT ? 0x7f : 0xff); > } else { > CMOS_DPRINTF("cmos: write index=0x%02x val=0x%02x\n", > s->cmos_index, data); > @@ -581,7 +585,6 @@ static void rtc_reset(void *opaque) > static int rtc_initfn(ISADevice *dev) > { > RTCState *s = DO_UPCAST(RTCState, dev, dev); > - int base = 0x70; > > s->cmos_data[RTC_REG_A] = 0x26; > s->cmos_data[RTC_REG_B] = 0x02; > @@ -603,14 +606,57 @@ static int rtc_initfn(ISADevice *dev) > qemu_get_clock(rtc_clock) + (get_ticks_per_sec() * 99) / 100; > qemu_mod_timer(s->second_timer2, s->next_second_time); > > - register_ioport_write(base, 2, 1, cmos_ioport_write, s); > - register_ioport_read(base, 2, 1, cmos_ioport_read, s); > + register_ioport_write(BASE_PORT, 4, 1, cmos_ioport_write, s); > + register_ioport_read(BASE_PORT, 4, 1, cmos_ioport_read, s); > > - qdev_set_legacy_instance_id(&dev->qdev, base, 2); > + qdev_set_legacy_instance_id(&dev->qdev, BASE_PORT, 2); > qemu_register_reset(rtc_reset, s); > return 0; > } > > +static long get_file_size(FILE *f) > +{ > + long where, size; > + > + /* XXX: on Unix systems, using fstat() probably makes more sense */ > + > + where = ftell(f); > + fseek(f, 0, SEEK_END); > + size = ftell(f); > + fseek(f, where, SEEK_SET); > + > + return size; > +} > BlockDrivers have a getlength() functions. Regards, Anthony Liguori
Am 16.09.2010 18:49, Stefan Weil schrieb: > The intention of this patch is ok. Loading CMOS with initial data > is needed. I just want to add two questions / remarks how the > implementation might be improved. > > Are there use cases where having a smaller CMOS size is better? > For example, when I want to emulate a system with 128 byte CMOS? > The size of CMOS could be derived from the size of the data > or specified by an additional parameter. I cannot image cases where smaller sizes would be a benefit. The saved disk space is negligible and you can always pad your CMOS configuration uo to 256 bytes by filling the gap with zero bytes. If the system just accesses the first 128 bytes the additional 128 zero bytes shouldn't hurt. > Using QEMU's block devices instead of a simple file would be > more consistent with the rest of QEMU and allow reading the > CMOS data not only from a file but also from an URL or other > sources. Thanks for the hint. Since this is my first contribution to the project I'm not that familiar with the code. Looking at other places, e.g. how the -kernel option gets handled, I just see FILE everywhere. Can you give me some pointers how to use this interface? Thanks for the review! Mathias
Am 16.09.2010 19:20 schrieb Anthony Liguori: > Instead of using FILE, I'd suggest using a BlockDriver to read and write > the data. I'll fix that as soon as I figured how to use this interface. > I think it would be very nice to add write support too so that writes to > CMOS were persisted across boots. Indeed. Also I would like to have a command line interface like '-cmos cmos.bin' instead of the ugly '-global mc146818rtc.file=cmos.bin'. But I'm not aware how to create such an alias. Any pointers? >> +static long get_file_size(FILE *f) >> +{ >> + long where, size; >> + >> + /* XXX: on Unix systems, using fstat() probably makes more sense */ >> + >> + where = ftell(f); >> + fseek(f, 0, SEEK_END); >> + size = ftell(f); >> + fseek(f, where, SEEK_SET); >> + >> + return size; >> +} >> > > BlockDrivers have a getlength() functions. Would reduce the size of the patch which is always a good thing (tm). Mathias
Hi Mathias, Am 17.09.2010 08:42, schrieb Mathias Krause: >> Using QEMU's block devices instead of a simple file would be >> more consistent with the rest of QEMU and allow reading the >> CMOS data not only from a file but also from an URL or other >> sources. > > Thanks for the hint. Since this is my first contribution to the project > I'm not that familiar with the code. Looking at other places, e.g. how > the -kernel option gets handled, I just see FILE everywhere. Can you > give me some pointers how to use this interface? Have a look at block.h which contains the prototypes for the public block layer interface. Basically, you need to create a BlockDriverState with bdrv_new() and then open it with bdrv_open(). You'll want to specify the raw block driver for opening the image, you get it with bdrv_find_format("raw"). bdrv_pread/pwrite are the right functions to access the file with byte granularity (other functions work on 512 byte sectors). bdrv_delete frees the the BlockDriverState when you're done. HTH, Kevin
Am 17.09.2010 08:42, schrieb Mathias Krause: > Am 16.09.2010 18:49, Stefan Weil schrieb: >> The intention of this patch is ok. Loading CMOS with initial data >> is needed. I just want to add two questions / remarks how the >> implementation might be improved. >> >> Are there use cases where having a smaller CMOS size is better? >> For example, when I want to emulate a system with 128 byte CMOS? >> The size of CMOS could be derived from the size of the data >> or specified by an additional parameter. > > I cannot image cases where smaller sizes would be a benefit. The saved > disk space is negligible and you can always pad your CMOS configuration > uo to 256 bytes by filling the gap with zero bytes. If the system just > accesses the first 128 bytes the additional 128 zero bytes shouldn't hurt. > The benefit of smaller sizes is not saving RAM but precision of the emulation. A BIOS or an operating system might be designed to support CMOS with 128 bytes or 256 bytes and try guessing the size by probing (many systems do something like this for RAM or other memory types). Testing the support of the small CMOS will be impossible if the emulation always emulates 256 bytes. Filling the additional 128 bytes with zero bytes won't help because the system will access them successfully although writes should fail. >> Using QEMU's block devices instead of a simple file would be >> more consistent with the rest of QEMU and allow reading the >> CMOS data not only from a file but also from an URL or other >> sources. > > Thanks for the hint. Since this is my first contribution to the project > I'm not that familiar with the code. Looking at other places, e.g. how > the -kernel option gets handled, I just see FILE everywhere. Can you > give me some pointers how to use this interface? > > Thanks for the review! > > > Mathias >
On 17.09.2010 12:58, Stefan Weil wrote: > Am 17.09.2010 08:42, schrieb Mathias Krause: >> Am 16.09.2010 18:49, Stefan Weil schrieb: >>> Are there use cases where having a smaller CMOS size is better? >>> For example, when I want to emulate a system with 128 byte CMOS? >>> The size of CMOS could be derived from the size of the data >>> or specified by an additional parameter. >> >> I cannot image cases where smaller sizes would be a benefit. The saved >> disk space is negligible and you can always pad your CMOS configuration >> uo to 256 bytes by filling the gap with zero bytes. If the system just >> accesses the first 128 bytes the additional 128 zero bytes shouldn't >> hurt. >> > > The benefit of smaller sizes is not saving RAM but precision > of the emulation. Well, as mentioned in my initial email the current emulation isn't accurate as well. It is emulating 128 bytes of CMOS RAM instead of 64 bytes, the real MC146818 only has. > A BIOS or an operating system might be designed to support > CMOS with 128 bytes or 256 bytes and try guessing the size > by probing (many systems do something like this for RAM or > other memory types). > > Testing the support of the small CMOS will be impossible > if the emulation always emulates 256 bytes. Filling the > additional 128 bytes with zero bytes won't help because > the system will access them successfully although writes > should fail. To access the upper 128 bytes of memory the operation system must be aware that it needs to use different I/O ports to probe for those addresses. The patch doesn't change the old behavior. Only the lower 128 bytes of memory will be accessible through the default RTC ports. So if the operating system or BIOS in question already is aware of the alternative method to access the upper 128 bytes, why should if fail then?
Hi Kevin, On 17.09.2010 12:44, Kevin Wolf wrote: > Hi Mathias, > > Am 17.09.2010 08:42, schrieb Mathias Krause: >>> Using QEMU's block devices instead of a simple file would be >>> more consistent with the rest of QEMU and allow reading the >>> CMOS data not only from a file but also from an URL or other >>> sources. >> Thanks for the hint. Since this is my first contribution to the project >> I'm not that familiar with the code. Looking at other places, e.g. how >> the -kernel option gets handled, I just see FILE everywhere. Can you >> give me some pointers how to use this interface? > > Have a look at block.h which contains the prototypes for the public > block layer interface. > > Basically, you need to create a BlockDriverState with bdrv_new() and > then open it with bdrv_open(). You'll want to specify the raw block > driver for opening the image, you get it with bdrv_find_format("raw"). > bdrv_pread/pwrite are the right functions to access the file with byte > granularity (other functions work on 512 byte sectors). bdrv_delete > frees the the BlockDriverState when you're done. Thank you for the detailed writeup. I think I should figure out how to use it myself now. Albeit there seem to be not many users of this interface right now. Looks like it's currently only used for storage devices. So I'm questioning myself: What _real_ benefit would it bring to use the QEMU block device layer for the CMOS file? Cheers, Mathias
On 09/17/2010 01:50 AM, Mathias Krause wrote: > Am 16.09.2010 19:20 schrieb Anthony Liguori: > >> Instead of using FILE, I'd suggest using a BlockDriver to read and write >> the data. >> > I'll fix that as soon as I figured how to use this interface. > > >> I think it would be very nice to add write support too so that writes to >> CMOS were persisted across boots. >> > Indeed. Also I would like to have a command line interface like '-cmos > cmos.bin' instead of the ugly '-global mc146818rtc.file=cmos.bin'. But > I'm not aware how to create such an alias. Any pointers? > Unfortunately, it's a little complicated although it should get better in the future. The right way to do this today would be: -drive file=cmos.bin,if=none,id=nvram -global mc146818rtc.drive=nvram The use of -drive is historic. We'll have a better option in the future that will look something like: -blockdev file=cmos.bin,id=nvram -global mc146818rtc.drive=nvram But in either case, I'd suggest adding an -nvram option that was: -nvram <filename> Which would do: drive_add(optarg, "if=none,id=nvram"); And then in the RTC code, default drive to nvram. It gets a little tough to handle the case of in memory CMOS. Regards, Anthony Liguori > >>> +static long get_file_size(FILE *f) >>> +{ >>> + long where, size; >>> + >>> + /* XXX: on Unix systems, using fstat() probably makes more sense */ >>> + >>> + where = ftell(f); >>> + fseek(f, 0, SEEK_END); >>> + size = ftell(f); >>> + fseek(f, where, SEEK_SET); >>> + >>> + return size; >>> +} >>> >>> >> BlockDrivers have a getlength() functions. >> > Would reduce the size of the patch which is always a good thing (tm). > > > Mathias >
On 17.09.2010 15:27, Anthony Liguori wrote: > On 09/17/2010 01:50 AM, Mathias Krause wrote: >> Am 16.09.2010 19:20 schrieb Anthony Liguori: >> >>> Instead of using FILE, I'd suggest using a BlockDriver to read and write >>> the data. >>> >> I'll fix that as soon as I figured how to use this interface. >> >> >>> I think it would be very nice to add write support too so that writes to >>> CMOS were persisted across boots. >>> >> Indeed. Also I would like to have a command line interface like '-cmos >> cmos.bin' instead of the ugly '-global mc146818rtc.file=cmos.bin'. But >> I'm not aware how to create such an alias. Any pointers? >> > > Unfortunately, it's a little complicated although it should get better > in the future. The right way to do this today would be: > > -drive file=cmos.bin,if=none,id=nvram -global mc146818rtc.drive=nvram > > The use of -drive is historic. We'll have a better option in the future > that will look something like: > > -blockdev file=cmos.bin,id=nvram -global mc146818rtc.drive=nvram Well, I guess "better" lies in the eye of the beholder, then ;) > But in either case, I'd suggest adding an -nvram option that was: > > -nvram <filename> > > Which would do: > > drive_add(optarg, "if=none,id=nvram"); > > And then in the RTC code, default drive to nvram. I managed to add the nvram option but how do I get a reference to the drive back in the RTC code? Would I just loop with drive_get(IF_NONE, 0, i) until the id of the returned drive is "nvram"? Doesn't sound right but I've found no better solution due to the lack of an drive_get_by_id() function. > It gets a little tough to handle the case of in memory CMOS. I don't quite get your point here. The content of the drive is copied in the RTC init routine into a private char array. How could the type of the drive may be a problem here? Regards, Mathias
On 09/22/2010 09:43 PM, Mathias Krause wrote: > I managed to add the nvram option but how do I get a reference to the > drive back in the RTC code? Would I just loop with drive_get(IF_NONE, 0, > i) until the id of the returned drive is "nvram"? Doesn't sound right > but I've found no better solution due to the lack of an > drive_get_by_id() function. You can write one. :) It's going to be very similar to drive_get_by_blockdev. Paolo
Mathias Krause <mathias.krause@secunet.com> writes: > Hi Kevin, > > On 17.09.2010 12:44, Kevin Wolf wrote: >> Hi Mathias, >> >> Am 17.09.2010 08:42, schrieb Mathias Krause: >>>> Using QEMU's block devices instead of a simple file would be >>>> more consistent with the rest of QEMU and allow reading the >>>> CMOS data not only from a file but also from an URL or other >>>> sources. >>> Thanks for the hint. Since this is my first contribution to the project >>> I'm not that familiar with the code. Looking at other places, e.g. how >>> the -kernel option gets handled, I just see FILE everywhere. Can you >>> give me some pointers how to use this interface? >> >> Have a look at block.h which contains the prototypes for the public >> block layer interface. >> >> Basically, you need to create a BlockDriverState with bdrv_new() and >> then open it with bdrv_open(). You'll want to specify the raw block >> driver for opening the image, you get it with bdrv_find_format("raw"). >> bdrv_pread/pwrite are the right functions to access the file with byte >> granularity (other functions work on 512 byte sectors). bdrv_delete >> frees the the BlockDriverState when you're done. > > Thank you for the detailed writeup. I think I should figure out how to > use it myself now. Albeit there seem to be not many users of this > interface right now. Looks like it's currently only used for storage > devices. So I'm questioning myself: What _real_ benefit would it bring > to use the QEMU block device layer for the CMOS file? I'd view initial CMOS contents as configuration. We don't use the block layer to access configuration files.
Anthony Liguori <anthony@codemonkey.ws> writes: > On 09/16/2010 08:58 AM, Mathias Krause wrote: >> In contrast to the BIOS and Option ROMs the CMOS content cannot be >> predefined by the user. Also the amount of useable CMOS ARM is pretty >> limited, even though the amount of CMOS bytes emulated by qemu is >> already twice as much as of the original MC146818. Nevertheless those >> limitations are pretty annoying when testing different BIOS replacement >> implementations like coreboot in combination with FILO that use CMOS >> values above the basic RTC range for its own purpose to, e.g., control >> the boot device order using a string containing the boot devices to >> probe. >> >> This patch adds support to specify a file to read the initial CMOS >> content from. It also increases the CMOS size to 256 bytes and >> implements access to the extended memory range via I/O ports 0x72 and >> 0x73. >> >> Use it like: `qemu -global mc146818rtc.file=cmos.bin ...' where cmos.bin >> is a binary file, sized 256 bytes containing the CMOS RAM. >> >> Signed-off-by: Mathias Krause<mathias.krause@secunet.com> >> > > Instead of using FILE, I'd suggest using a BlockDriver to read and > write the data. > > I think it would be very nice to add write support too so that writes > to CMOS were persisted across boots. Persisting CMOS makes it state instead of configuration. Use of block layer (treat it like a drive) makes more sense then.
Mathias Krause <mathias.krause@secunet.com> writes: > On 17.09.2010 15:27, Anthony Liguori wrote: >> On 09/17/2010 01:50 AM, Mathias Krause wrote: >>> Am 16.09.2010 19:20 schrieb Anthony Liguori: >>> >>>> Instead of using FILE, I'd suggest using a BlockDriver to read and write >>>> the data. >>>> >>> I'll fix that as soon as I figured how to use this interface. >>> >>> >>>> I think it would be very nice to add write support too so that writes to >>>> CMOS were persisted across boots. >>>> >>> Indeed. Also I would like to have a command line interface like '-cmos >>> cmos.bin' instead of the ugly '-global mc146818rtc.file=cmos.bin'. But >>> I'm not aware how to create such an alias. Any pointers? >>> >> >> Unfortunately, it's a little complicated although it should get better >> in the future. The right way to do this today would be: >> >> -drive file=cmos.bin,if=none,id=nvram -global mc146818rtc.drive=nvram >> >> The use of -drive is historic. We'll have a better option in the future >> that will look something like: >> >> -blockdev file=cmos.bin,id=nvram -global mc146818rtc.drive=nvram > > Well, I guess "better" lies in the eye of the beholder, then ;) > > >> But in either case, I'd suggest adding an -nvram option that was: >> >> -nvram <filename> >> >> Which would do: >> >> drive_add(optarg, "if=none,id=nvram"); >> >> And then in the RTC code, default drive to nvram. > > I managed to add the nvram option but how do I get a reference to the > drive back in the RTC code? Would I just loop with drive_get(IF_NONE, 0, > i) until the id of the returned drive is "nvram"? Doesn't sound right > but I've found no better solution due to the lack of an > drive_get_by_id() function. Use DEFINE_PROP_DRIVE() to define mc146818rtc's property drive, and it's automatic: qdev assigns a pointer to the BlockDriverState. To access DriveInfo, use drive_get_by_blockdev(). I doubt you need that. [...]
On 24.09.2010 14:47, Markus Armbruster wrote: > Mathias Krause <mathias.krause@secunet.com> writes: > >> On 17.09.2010 15:27, Anthony Liguori wrote: >>> On 09/17/2010 01:50 AM, Mathias Krause wrote: >>>> Am 16.09.2010 19:20 schrieb Anthony Liguori: >>>> >>>>> Instead of using FILE, I'd suggest using a BlockDriver to read and write >>>>> the data. >>>>> >>>> I'll fix that as soon as I figured how to use this interface. >>>> >>>> >>>>> I think it would be very nice to add write support too so that writes to >>>>> CMOS were persisted across boots. >>>>> >>>> Indeed. Also I would like to have a command line interface like '-cmos >>>> cmos.bin' instead of the ugly '-global mc146818rtc.file=cmos.bin'. But >>>> I'm not aware how to create such an alias. Any pointers? >>>> >>> Unfortunately, it's a little complicated although it should get better >>> in the future. The right way to do this today would be: >>> >>> -drive file=cmos.bin,if=none,id=nvram -global mc146818rtc.drive=nvram >>> >>> The use of -drive is historic. We'll have a better option in the future >>> that will look something like: >>> >>> -blockdev file=cmos.bin,id=nvram -global mc146818rtc.drive=nvram >> Well, I guess "better" lies in the eye of the beholder, then ;) >> >> >>> But in either case, I'd suggest adding an -nvram option that was: >>> >>> -nvram <filename> >>> >>> Which would do: >>> >>> drive_add(optarg, "if=none,id=nvram"); >>> >>> And then in the RTC code, default drive to nvram. >> I managed to add the nvram option but how do I get a reference to the >> drive back in the RTC code? Would I just loop with drive_get(IF_NONE, 0, >> i) until the id of the returned drive is "nvram"? Doesn't sound right >> but I've found no better solution due to the lack of an >> drive_get_by_id() function. > > Use DEFINE_PROP_DRIVE() to define mc146818rtc's property drive, and it's > automatic: qdev assigns a pointer to the BlockDriverState. That works quite well but now, by using the QEMU block layer, the file for the CMOS data must be a multiple of 512 bytes. Otherwise bdrv_pread() will fail because the driver only handles full blocks so truncates the 256 bytes to a block device with 0 blocks. Sadly the block size of 512 seems to be hardcoded or can it be changed on a per drive basis? Regards, Mathias
Maybe Kevin or Christoph (cc'ed) can help. Mathias Krause <mathias.krause@secunet.com> writes: > On 24.09.2010 14:47, Markus Armbruster wrote: >> Mathias Krause <mathias.krause@secunet.com> writes: >> >>> On 17.09.2010 15:27, Anthony Liguori wrote: >>>> On 09/17/2010 01:50 AM, Mathias Krause wrote: >>>>> Am 16.09.2010 19:20 schrieb Anthony Liguori: >>>>> >>>>>> Instead of using FILE, I'd suggest using a BlockDriver to read and write >>>>>> the data. >>>>>> >>>>> I'll fix that as soon as I figured how to use this interface. >>>>> >>>>> >>>>>> I think it would be very nice to add write support too so that writes to >>>>>> CMOS were persisted across boots. >>>>>> >>>>> Indeed. Also I would like to have a command line interface like '-cmos >>>>> cmos.bin' instead of the ugly '-global mc146818rtc.file=cmos.bin'. But >>>>> I'm not aware how to create such an alias. Any pointers? >>>>> >>>> Unfortunately, it's a little complicated although it should get better >>>> in the future. The right way to do this today would be: >>>> >>>> -drive file=cmos.bin,if=none,id=nvram -global mc146818rtc.drive=nvram >>>> >>>> The use of -drive is historic. We'll have a better option in the future >>>> that will look something like: >>>> >>>> -blockdev file=cmos.bin,id=nvram -global mc146818rtc.drive=nvram >>> Well, I guess "better" lies in the eye of the beholder, then ;) >>> >>> >>>> But in either case, I'd suggest adding an -nvram option that was: >>>> >>>> -nvram <filename> >>>> >>>> Which would do: >>>> >>>> drive_add(optarg, "if=none,id=nvram"); >>>> >>>> And then in the RTC code, default drive to nvram. >>> I managed to add the nvram option but how do I get a reference to the >>> drive back in the RTC code? Would I just loop with drive_get(IF_NONE, 0, >>> i) until the id of the returned drive is "nvram"? Doesn't sound right >>> but I've found no better solution due to the lack of an >>> drive_get_by_id() function. >> >> Use DEFINE_PROP_DRIVE() to define mc146818rtc's property drive, and it's >> automatic: qdev assigns a pointer to the BlockDriverState. > > That works quite well but now, by using the QEMU block layer, the file > for the CMOS data must be a multiple of 512 bytes. Otherwise > bdrv_pread() will fail because the driver only handles full blocks so > truncates the 256 bytes to a block device with 0 blocks. Sadly the block > size of 512 seems to be hardcoded or can it be changed on a per drive basis?
diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c index 2b91fa8..9f215e0 100644 --- a/hw/mc146818rtc.c +++ b/hw/mc146818rtc.c @@ -78,12 +78,16 @@ #define REG_C_PF 0x40 #define REG_C_AF 0x20 +#define BASE_PORT 0x70 +#define CMOS_SIZE 256 + typedef struct RTCState { ISADevice dev; - uint8_t cmos_data[128]; + uint8_t cmos_data[CMOS_SIZE]; uint8_t cmos_index; struct tm current_tm; int32_t base_year; + char *file; qemu_irq irq; qemu_irq sqw_irq; int it_shift; @@ -206,7 +210,7 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data) RTCState *s = opaque; if ((addr & 1) == 0) { - s->cmos_index = data & 0x7f; + s->cmos_index = data & (addr == BASE_PORT ? 0x7f : 0xff); } else { CMOS_DPRINTF("cmos: write index=0x%02x val=0x%02x\n", s->cmos_index, data); @@ -581,7 +585,6 @@ static void rtc_reset(void *opaque) static int rtc_initfn(ISADevice *dev) { RTCState *s = DO_UPCAST(RTCState, dev, dev); - int base = 0x70; s->cmos_data[RTC_REG_A] = 0x26; s->cmos_data[RTC_REG_B] = 0x02; @@ -603,14 +606,57 @@ static int rtc_initfn(ISADevice *dev) qemu_get_clock(rtc_clock) + (get_ticks_per_sec() * 99) / 100; qemu_mod_timer(s->second_timer2, s->next_second_time); - register_ioport_write(base, 2, 1, cmos_ioport_write, s); - register_ioport_read(base, 2, 1, cmos_ioport_read, s); + register_ioport_write(BASE_PORT, 4, 1, cmos_ioport_write, s); + register_ioport_read(BASE_PORT, 4, 1, cmos_ioport_read, s); - qdev_set_legacy_instance_id(&dev->qdev, base, 2); + qdev_set_legacy_instance_id(&dev->qdev, BASE_PORT, 2); qemu_register_reset(rtc_reset, s); return 0; } +static long get_file_size(FILE *f) +{ + long where, size; + + /* XXX: on Unix systems, using fstat() probably makes more sense */ + + where = ftell(f); + fseek(f, 0, SEEK_END); + size = ftell(f); + fseek(f, where, SEEK_SET); + + return size; +} + +static void cmos_load_from_file(RTCState *s, const char *file) +{ + const char *err_msg = NULL; + FILE *f; + + if (!(f = fopen(file, "r"))) { + err_msg = strerror(errno); + goto err; + } + + if (get_file_size(f) != CMOS_SIZE) { + err_msg = "invalid file size"; + goto err; + } + + if (fread(s->cmos_data, 1, CMOS_SIZE, f) != CMOS_SIZE) { + err_msg = strerror(errno); + goto err; + } + CMOS_DPRINTF("cmos: initialized from '%s'\n", file); + + fclose(f); + + return; +err: + fprintf(stderr, "qemu: could not load CMOS '%s': %s\n", file, err_msg); + exit(1); +} + ISADevice *rtc_init(int base_year, qemu_irq intercept_irq) { ISADevice *dev; @@ -618,6 +664,9 @@ ISADevice *rtc_init(int base_year, qemu_irq intercept_irq) dev = isa_create("mc146818rtc"); s = DO_UPCAST(RTCState, dev, dev); + if (s->file != NULL) { + cmos_load_from_file(s, s->file); + } qdev_prop_set_int32(&dev->qdev, "base_year", base_year); qdev_init_nofail(&dev->qdev); if (intercept_irq) { @@ -636,6 +685,7 @@ static ISADeviceInfo mc146818rtc_info = { .init = rtc_initfn, .qdev.props = (Property[]) { DEFINE_PROP_INT32("base_year", RTCState, base_year, 1980), + DEFINE_PROP_STRING("file", RTCState, file), DEFINE_PROP_END_OF_LIST(), } };
In contrast to the BIOS and Option ROMs the CMOS content cannot be predefined by the user. Also the amount of useable CMOS ARM is pretty limited, even though the amount of CMOS bytes emulated by qemu is already twice as much as of the original MC146818. Nevertheless those limitations are pretty annoying when testing different BIOS replacement implementations like coreboot in combination with FILO that use CMOS values above the basic RTC range for its own purpose to, e.g., control the boot device order using a string containing the boot devices to probe. This patch adds support to specify a file to read the initial CMOS content from. It also increases the CMOS size to 256 bytes and implements access to the extended memory range via I/O ports 0x72 and 0x73. Use it like: `qemu -global mc146818rtc.file=cmos.bin ...' where cmos.bin is a binary file, sized 256 bytes containing the CMOS RAM. Signed-off-by: Mathias Krause <mathias.krause@secunet.com> --- hw/mc146818rtc.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 56 insertions(+), 6 deletions(-)