SeaBIOS

seabios@seabios.org

4 participants
3801 discussions

[PATCH 4/4] bochsvga: validate EDID resolution in init
by Hiroshi Miura 25 Oct '12

25 Oct '12

As same as bochsvga that init code disables modes that is larger than max_{x|y}res, bochsedid also disables STD_EDID that is larger than max_{x|y}res. The int15h returns largest and smaller edid mode than MAX_XRES, MAX_YRES. Signed-off-by: Hiroshi Miura <miurahr(a)linux.com> --- vgasrc/bochsedid.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++--- vgasrc/bochsedid.h | 8 +++- vgasrc/bochsvga.c | 10 ++++- 3 files changed, 113 insertions(+), 9 deletions(-) diff --git a/vgasrc/bochsedid.c b/vgasrc/bochsedid.c index ec43d41..cf90c04 100644 --- a/vgasrc/bochsedid.c +++ b/vgasrc/bochsedid.c @@ -10,10 +10,54 @@ #include "config.h" // CONFIG_* #include "bochsedid.h" -struct edid_detailed_timing edid_dtd_1920x1200 VAR16 = { - 0x4B7D, 0xB0, 0xA0, 0x72, 0xB0, 0x2D, +static struct bochs_edid_mode { + u16 mode; + u16 width; + u16 height; + u8 ratio; +} bochs_edid_modes[] VAR16 = { + {EDID_STD_1920x1200_60Hz,1920,1200, EDID_STD_RATIO_16_10}, + {EDID_STD_1920x1080_60Hz,1920,1080, EDID_STD_RATIO_16_9}, + {EDID_STD_1680x1050_60Hz,1680,1050, EDID_STD_RATIO_16_10}, + {EDID_STD_1600x1200_60Hz,1600,1200, EDID_STD_RATIO_4_3}, + {EDID_STD_1600x900_60Hz, 1600, 900, EDID_STD_RATIO_16_9}, + {EDID_STD_1440x900_60Hz, 1440, 900, EDID_STD_RATIO_16_10}, + {EDID_STD_1280x1024_60Hz,1280,1024, EDID_STD_RATIO_5_4}, + {EDID_STD_1280x960_60Hz, 1280, 960, EDID_STD_RATIO_4_3}, + {EDID_STD_1152x864_70Hz, 1152, 864, EDID_STD_RATIO_4_3}, + {EDID_STD_1024x768_85Hz, 1024, 768, EDID_STD_RATIO_4_3}, + {EDID_STD_800x600_85Hz, 800, 600, EDID_STD_RATIO_4_3}, + {EDID_STD_640x480_85Hz, 640, 480, EDID_STD_RATIO_4_3} +}; + +struct bochs_edid_dtd { + struct edid_detailed_timing dtd; + u16 width; + u16 height; + u8 ratio; + int disabled; +} bochs_edid_dtds[] VAR16 = { + {{0x4B7D, 0xB0, 0xA0, 0x72, 0xB0, 0x2D, 0x40, 0x88, 0xC8, 0x55, 0x75, 0x21, - 0x00, 0x00, 0x1C}; + 0x00, 0x00, 0x1C}, + 1920,1200,EDID_STD_RATIO_16_10,0 + }, + {{0x3c8c, 0x40, 0x30, 0x62, 0xB0, 0x32, + 0x40, 0x40, 0xC0, 0x13, 0x00, 0x2C, 0xE1, 0x10, + 0x00, 0x00, 0x1E}, + 1600,1200,EDID_STD_RATIO_4_3,0 + }, + {{0x2a30, 0x00, 0x98, 0x51, 0x00, 0x2A, + 0x40, 0x30, 0x70, 0x13, 0x00, 0x2C, 0xE1, 0x10, + 0x00, 0x00, 0x1E}, + 1280,1024,EDID_STD_RATIO_5_4,0 + }, + {{0x2a30, 0x80, 0xC0, 0x41, 0x60, 0x24, + 0x30, 0x40, 0x80, 0x13, 0x00, 0x2C, 0xE1, 0x10, + 0x00, 0x00, 0x1E}, + 1152, 864,EDID_STD_RATIO_4_3,0 + }, +}; struct edid_monitor_range_limit edid_range_limit VAR16 = { {0x00,0x00,0x00,0xfD,0x00}, 50,150, /* Vertical 50Hz - 150Hz */ @@ -58,11 +102,58 @@ struct edid_info BochsEDID VAR16 = { EDID_STD_1600x1200_60Hz,EDID_STD_NOP}, }; -void bochs_pupulate_edid_block0(u8 next) +void bochs_validate_edid_resolutions(u16 xres, u16 yres) { + struct bochs_edid_mode *m = bochs_edid_modes; + for (; m < &bochs_edid_modes[ARRAY_SIZE(bochs_edid_modes)]; m++){ + u16 width = GET_GLOBAL(m->width); + u16 height = GET_GLOBAL(m->height); + if (width > xres || height > yres) { + dprintf(1, "Removing edid mode %x\n", GET_GLOBAL(m->mode)); + SET_VGA(m->mode, EDID_STD_NOP); + } + } + struct bochs_edid_dtd *d = bochs_edid_dtds; + for (; d < &bochs_edid_dtds[ARRAY_SIZE(bochs_edid_dtds)]; d++){ + u16 width = GET_GLOBAL(d->width); + u16 height = GET_GLOBAL(d->height); + if (width > xres || height > yres) { + dprintf(1, "Removing edid dtd %d x %d\n", width, height); + SET_VGA(d->disabled, 1); + } + } +} + +int bochs_pupulate_edid_block0(u8 next) { + int i; + + struct bochs_edid_mode *m = bochs_edid_modes; + for (i = 0; (i < 8) && (m < &bochs_edid_modes[ARRAY_SIZE(bochs_edid_modes)]); m++){ + if (GET_GLOBAL(m->mode) != EDID_STD_NOP) { + SET_VGA(BochsEDID.standard_timing[i], GET_GLOBAL(m->mode)); + i++; + } + } + for (; i < 8; i++) { /* if rest, fill with NOP */ + SET_VGA(BochsEDID.standard_timing[i], EDID_STD_NOP); + } /* detailed timing block */ - memcpy_far(get_global_seg(), &(BochsEDID.desc[0]), get_global_seg(), - &edid_dtd_1920x1200, sizeof (struct edid_detailed_timing)); + /* select one that is largest and lower than max_xres */ + int done = 0; + struct bochs_edid_dtd *d = bochs_edid_dtds; + for (; d < &bochs_edid_dtds[ARRAY_SIZE(bochs_edid_dtds)]; d++){ + if (d->disabled) { + continue; + } + /* copy first entry within max_resolution */ + memcpy_far(get_global_seg(), &(BochsEDID.desc[0]), get_global_seg(), + &(d->dtd), sizeof (struct edid_detailed_timing)); + done = 1; + break; + } + if (done == 0) + return -1; + /* monitor range */ memcpy_far(get_global_seg(), &(BochsEDID.desc[1].mrld), get_global_seg(), &edid_range_limit, sizeof(edid_range_limit)); @@ -76,6 +167,7 @@ void bochs_pupulate_edid_block0(u8 next) SET_VGA(BochsEDID.extensions, next); u8 sum = -checksum_far(get_global_seg(), &BochsEDID, sizeof(BochsEDID)); SET_VGA(BochsEDID.checksum, sum); + return 0; } int bochs_read_edid_block0(u16 unit, u16 block, u16 seg, void *data) diff --git a/vgasrc/bochsedid.h b/vgasrc/bochsedid.h index b6b2e3a..60ae3c9 100644 --- a/vgasrc/bochsedid.h +++ b/vgasrc/bochsedid.h @@ -131,7 +131,13 @@ struct edid_info { #define EDID_STD_1920x1200_60Hz 0x00D1 #define EDID_STD_NOP 0x0101 -void bochs_pupulate_edid_block0(u8 next); +#define EDID_STD_RATIO_16_10 (1) +#define EDID_STD_RATIO_4_3 (1 << 1) +#define EDID_STD_RATIO_5_4 (1 << 2) +#define EDID_STD_RATIO_16_9 (1 << 3) + +int bochs_pupulate_edid_block0(u8 next); +void bochs_validate_edid_resolutions(u16 xres, u16 yres); int bochs_get_ddc_capabilities(u16 unit); int bochs_read_edid(u16 unit, u16 block, u16 seg, void *data); #endif /* bochedid.h */ diff --git a/vgasrc/bochsvga.c b/vgasrc/bochsvga.c index 7b8f430..bcb41be 100644 --- a/vgasrc/bochsvga.c +++ b/vgasrc/bochsvga.c @@ -369,12 +369,14 @@ bochsvga_init(void) dprintf(1, "VBE DISPI: lfb_addr=%x, size %d MB\n", lfb_addr, totalmem >> 20); - // Validate modes + // Validate modes and edid resolutions u16 en = dispi_read(VBE_DISPI_INDEX_ENABLE); dispi_write(VBE_DISPI_INDEX_ENABLE, en | VBE_DISPI_GETCAPS); u16 max_xres = dispi_read(VBE_DISPI_INDEX_XRES); + u16 max_yres = dispi_read(VBE_DISPI_INDEX_YRES); u16 max_bpp = dispi_read(VBE_DISPI_INDEX_BPP); dispi_write(VBE_DISPI_INDEX_ENABLE, en); + /* modes */ struct bochsvga_mode *m = bochsvga_modes; for (; m < &bochsvga_modes[ARRAY_SIZE(bochsvga_modes)]; m++) { u16 width = GET_GLOBAL(m->info.width); @@ -388,7 +390,11 @@ bochsvga_init(void) SET_VGA(m->mode, 0xffff); } } - bochs_pupulate_edid_block0(0); + /* edid resolutions */ + bochs_validate_edid_resolutions(max_xres, max_yres); + ret = bochs_pupulate_edid_block0(0); + if (ret) + return ret; return 0; } -- 1.7.9.5

1 0

[PATCH 2/4] add VGA_EDID staff
by Hiroshi Miura 25 Oct '12

25 Oct '12

Signed-off-by: Hiroshi Miura <miurahr(a)linux.com> --- vgasrc/Kconfig | 8 ++++++++ vgasrc/vbe.c | 5 +++++ 2 files changed, 13 insertions(+) diff --git a/vgasrc/Kconfig b/vgasrc/Kconfig index 6b22c39..42540e0 100644 --- a/vgasrc/Kconfig +++ b/vgasrc/Kconfig @@ -54,6 +54,14 @@ menu "VGA ROM" help Support VBE. + config VGA_EDID + depends on VGA_VBE + bool "VBE DDC EDID support" + default y + help + Support VBE Direct Display Connection(DDC) standard's. + Extended Display Information Descriptor(EDID). + config VGA_PCI depends on BUILD_VGABIOS bool "PCI ROM Headers" diff --git a/vgasrc/vbe.c b/vgasrc/vbe.c index ed7c6ee..423cbe1 100644 --- a/vgasrc/vbe.c +++ b/vgasrc/vbe.c @@ -392,6 +392,11 @@ vbe_104f15(struct bregs *regs) u16 unit = regs->cx; u16 block = regs->dx; + if (!CONFIG_VGA_EDID) { + regs->ax = 0x014f; + return; + } + switch (regs->bl) { case 0x00: /* Report VBE/DDC Capabilities */ ret = vgahw_get_ddc_capabilities(unit); -- 1.7.9.5

1 0

[PATCH 1/4] vgasrc: Add skeleton for VBE function 15h (VBE/DDC Standard)
by Hiroshi Miura 25 Oct '12

25 Oct '12

posted in Wed Mar 7 03:41:06 CET 2012 Signed-off-by: Julian Pidancet <julian.pidancet(a)gmail.com> Signed-off-by: Hiroshi Miura <miurahr(a)linux.com> --- src/vbe.h | 5 +++++ vgasrc/stdvga.c | 11 +++++++++++ vgasrc/stdvga.h | 2 ++ vgasrc/vbe.c | 34 ++++++++++++++++++++++++++++++++++ vgasrc/vgahw.h | 8 ++++++++ 5 files changed, 60 insertions(+) diff --git a/src/vbe.h b/src/vbe.h index 386613c..b5aedc6 100644 --- a/src/vbe.h +++ b/src/vbe.h @@ -154,4 +154,9 @@ struct vbe_crtc_info { #define VBE_DIRECTCOLOR_COLOR_RAMP_PROGRAMMABLE 0x01 #define VBE_DIRECTCOLOR_RESERVED_BITS_AVAILABLE 0x02 +/* Display Data Channel (DDC) */ +#define VBE_DDC1_PROTOCOL_SUPPORTED 0x01 +#define VBE_DDC2_PROTOCOL_SUPPORTED 0x02 +#define VBE_DDC_BLANK_DURING_TRANSFER 0x04 + #endif diff --git a/vgasrc/stdvga.c b/vgasrc/stdvga.c index ae6c0be..270cd65 100644 --- a/vgasrc/stdvga.c +++ b/vgasrc/stdvga.c @@ -499,3 +499,14 @@ stdvga_init(void) return 0; } + +int stdvga_get_ddc_capabilities(u16 unit) +{ + return -1; +} + +int stdvga_read_edid(u16 unit, u16 block, u16 seg, void *data) +{ + return -1; +} + diff --git a/vgasrc/stdvga.h b/vgasrc/stdvga.h index c9a9ba1..e3bd447 100644 --- a/vgasrc/stdvga.h +++ b/vgasrc/stdvga.h @@ -108,5 +108,7 @@ int stdvga_save_state(u16 seg, void *data, int states); int stdvga_restore_state(u16 seg, void *data, int states); void stdvga_enable_video_addressing(u8 disable); int stdvga_init(void); +int stdvga_get_ddc_capabilities(u16 unit); +int stdvga_read_edid(u16 unit, u16 block, u16 seg, void *data); #endif // stdvga.h diff --git a/vgasrc/vbe.c b/vgasrc/vbe.c index 227a244..ed7c6ee 100644 --- a/vgasrc/vbe.c +++ b/vgasrc/vbe.c @@ -384,6 +384,39 @@ vbe_104f10(struct bregs *regs) } static void +vbe_104f15(struct bregs *regs) +{ + int ret; + u16 seg = regs->es; + void *data = (void *)(regs->di+0); + u16 unit = regs->cx; + u16 block = regs->dx; + + switch (regs->bl) { + case 0x00: /* Report VBE/DDC Capabilities */ + ret = vgahw_get_ddc_capabilities(unit); + if (ret < 0) + goto fail; + regs->bh = ret >> 8; /* Approx. time in seconds, rounded up, to transfer + one EDID block (128 bytes) */ + regs->bl = ret & 0xff; /* DDC level supported */ + break; + case 0x01: /* Read EDID */ + ret = vgahw_read_edid(unit, block, seg, data); + if (ret < 0) + goto fail; + break; + default: + goto fail; + } + regs->ax = 0x004f; + return; + +fail: + regs->ax = 0x014f; +} + +static void vbe_104fXX(struct bregs *regs) { debug_stub(regs); @@ -410,6 +443,7 @@ handle_104f(struct bregs *regs) case 0x08: vbe_104f08(regs); break; case 0x0a: vbe_104f0a(regs); break; case 0x10: vbe_104f10(regs); break; + case 0x15: vbe_104f15(regs); break; default: vbe_104fXX(regs); break; } } diff --git a/vgasrc/vgahw.h b/vgasrc/vgahw.h index 044cd32..01d2609 100644 --- a/vgasrc/vgahw.h +++ b/vgasrc/vgahw.h @@ -129,4 +129,12 @@ static inline int vgahw_restore_state(u16 seg, void *data, int states) { return stdvga_restore_state(seg, data, states); } +static inline int vgahw_get_ddc_capabilities(u16 unit) { + return stdvga_get_ddc_capabilities(unit); +} + +static inline int vgahw_read_edid(u16 unit, u16 block, u16 seg, void *data) { + return stdvga_read_edid(unit, block, seg, data); +} + #endif // vgahw.h -- 1.7.9.5

1 0

Re: [SeaBIOS] [Qemu-devel] [RFC PATCH v3 05/19] Implement dimm device abstraction
by Vasilis Liaskovitis 24 Oct '12

24 Oct '12

Hi, On Wed, Oct 24, 2012 at 12:15:17PM +0200, Stefan Hajnoczi wrote: > On Wed, Oct 24, 2012 at 10:06 AM, liu ping fan <qemulist(a)gmail.com> wrote: > > On Tue, Oct 23, 2012 at 8:25 PM, Stefan Hajnoczi <stefanha(a)gmail.com> wrote: > >> On Fri, Sep 21, 2012 at 01:17:21PM +0200, Vasilis Liaskovitis wrote: > >>> +static void dimm_populate(DimmDevice *s) > >>> +{ > >>> + DeviceState *dev= (DeviceState*)s; > >>> + MemoryRegion *new = NULL; > >>> + > >>> + new = g_malloc(sizeof(MemoryRegion)); > >>> + memory_region_init_ram(new, dev->id, s->size); > >>> + vmstate_register_ram_global(new); > >>> + memory_region_add_subregion(get_system_memory(), s->start, new); > >>> + s->mr = new; > >>> +} > >>> + > >>> +static void dimm_depopulate(DimmDevice *s) > >>> +{ > >>> + assert(s); > >>> + vmstate_unregister_ram(s->mr, NULL); > >>> + memory_region_del_subregion(get_system_memory(), s->mr); > >>> + memory_region_destroy(s->mr); > >>> + s->mr = NULL; > >>> +} > >> > >> How is dimm hot unplug protected against callers who currently have RAM > >> mapped (from cpu_physical_memory_map())? > >> > >> Emulated devices call cpu_physical_memory_map() directly or indirectly > >> through DMA emulation code. The RAM pointer may be held for arbitrary > >> lengths of time, across main loop iterations, etc. > >> > >> It's not clear to me that it is safe to unplug a DIMM that has network > >> or disk I/O buffers, for example. We also need to be robust against > >> malicious guests who abuse the hotplug lifecycle. QEMU should never be > >> left with dangling pointers. > >> > > Not sure about the block layer. But I think those thread are already > > out of big lock, so there should be a MemoryListener to catch the > > RAM-unplug event, and if needed, bdrv_flush. do we want bdrv_flush, or some kind of cancel request e.g. bdrv_aio_cancel? > > Here is the detailed scenario: > > 1. Emulated device does cpu_physical_memory_map() and gets a pointer > to guest RAM. > 2. Return to vcpu or iothread, continue processing... > 3. Hot unplug of RAM causes the guest RAM to disappear. > 4. Pending I/O completes and overwrites memory from dangling guest RAM pointer. > > Any I/O device that does zero-copy I/O in QEMU faces this problem: > * The block layer is affected. > * The net layer is unaffected because it doesn't do zero-copy tx/rx > across returns to the main loop (#2 above). > * Not sure about other devices classes (e.g. USB). > > How should the MemoryListener callback work? For block I/O it may not > be possible to cancel pending I/O asynchronously - if you try to > cancel then your thread may block until the I/O completes. e.g. paio_cancel does this? is there already an API to asynchronously cancel all in flight operations in a BlockDriverState? Afaict block_job_cancel refers to streaming jobs only and doesn't help here. Can we make the RAM unplug initiate async I/O cancellations, prevent further I/Os, and only free the memory in a callback, after all DMA I/O to the associated memory region has been cancelled or completed? Also iiuc the MemoryListener should be registered from users of cpu_physical_memory_map e.g. hw/virtio.c By the way dimm_depopulate only frees the qemu memory on an ACPI _EJ request, which means that a well-behaved guest will have already offlined the memory and is not using it anymore. If the guest still uses the memory e.g. for a DMA buffer, the logical memory offlining will fail and the _EJ/qemu memory freeing will never happen. But in theory a malicious acpi guest driver could trigger _EJ requests to do step 3 above. Or perhaps the backing block driver can finish an I/O request for a zero-copy block device that the guest doesn't care for anymore? I 'll think about this a bit more. > Synchronous cancel behavior is not workable since it can lead to poor > latency or hangs in the guest. ok thanks, - Vasilis

1 0

Re: [SeaBIOS] [Qemu-devel] [RFC PATCH v3 05/19] Implement dimm device abstraction
by Stefan Hajnoczi 24 Oct '12

24 Oct '12

On Wed, Oct 24, 2012 at 10:06 AM, liu ping fan <qemulist(a)gmail.com> wrote: > On Tue, Oct 23, 2012 at 8:25 PM, Stefan Hajnoczi <stefanha(a)gmail.com> wrote: >> On Fri, Sep 21, 2012 at 01:17:21PM +0200, Vasilis Liaskovitis wrote: >>> +static void dimm_populate(DimmDevice *s) >>> +{ >>> + DeviceState *dev= (DeviceState*)s; >>> + MemoryRegion *new = NULL; >>> + >>> + new = g_malloc(sizeof(MemoryRegion)); >>> + memory_region_init_ram(new, dev->id, s->size); >>> + vmstate_register_ram_global(new); >>> + memory_region_add_subregion(get_system_memory(), s->start, new); >>> + s->mr = new; >>> +} >>> + >>> +static void dimm_depopulate(DimmDevice *s) >>> +{ >>> + assert(s); >>> + vmstate_unregister_ram(s->mr, NULL); >>> + memory_region_del_subregion(get_system_memory(), s->mr); >>> + memory_region_destroy(s->mr); >>> + s->mr = NULL; >>> +} >> >> How is dimm hot unplug protected against callers who currently have RAM >> mapped (from cpu_physical_memory_map())? >> >> Emulated devices call cpu_physical_memory_map() directly or indirectly >> through DMA emulation code. The RAM pointer may be held for arbitrary >> lengths of time, across main loop iterations, etc. >> >> It's not clear to me that it is safe to unplug a DIMM that has network >> or disk I/O buffers, for example. We also need to be robust against >> malicious guests who abuse the hotplug lifecycle. QEMU should never be >> left with dangling pointers. >> > Not sure about the block layer. But I think those thread are already > out of big lock, so there should be a MemoryListener to catch the > RAM-unplug event, and if needed, bdrv_flush. Here is the detailed scenario: 1. Emulated device does cpu_physical_memory_map() and gets a pointer to guest RAM. 2. Return to vcpu or iothread, continue processing... 3. Hot unplug of RAM causes the guest RAM to disappear. 4. Pending I/O completes and overwrites memory from dangling guest RAM pointer. Any I/O device that does zero-copy I/O in QEMU faces this problem: * The block layer is affected. * The net layer is unaffected because it doesn't do zero-copy tx/rx across returns to the main loop (#2 above). * Not sure about other devices classes (e.g. USB). How should the MemoryListener callback work? For block I/O it may not be possible to cancel pending I/O asynchronously - if you try to cancel then your thread may block until the I/O completes. Synchronous cancel behavior is not workable since it can lead to poor latency or hangs in the guest. Stefan

1 0

Re: [SeaBIOS] [Qemu-devel] [RFC PATCH v3 05/19] Implement dimm device abstraction
by Stefan Hajnoczi 24 Oct '12

24 Oct '12

On Fri, Sep 21, 2012 at 01:17:21PM +0200, Vasilis Liaskovitis wrote: > +static void dimm_populate(DimmDevice *s) > +{ > + DeviceState *dev= (DeviceState*)s; > + MemoryRegion *new = NULL; > + > + new = g_malloc(sizeof(MemoryRegion)); > + memory_region_init_ram(new, dev->id, s->size); > + vmstate_register_ram_global(new); > + memory_region_add_subregion(get_system_memory(), s->start, new); > + s->mr = new; > +} > + > +static void dimm_depopulate(DimmDevice *s) > +{ > + assert(s); > + vmstate_unregister_ram(s->mr, NULL); > + memory_region_del_subregion(get_system_memory(), s->mr); > + memory_region_destroy(s->mr); > + s->mr = NULL; > +} How is dimm hot unplug protected against callers who currently have RAM mapped (from cpu_physical_memory_map())? Emulated devices call cpu_physical_memory_map() directly or indirectly through DMA emulation code. The RAM pointer may be held for arbitrary lengths of time, across main loop iterations, etc. It's not clear to me that it is safe to unplug a DIMM that has network or disk I/O buffers, for example. We also need to be robust against malicious guests who abuse the hotplug lifecycle. QEMU should never be left with dangling pointers. Stefan

2 1

Re: [SeaBIOS] [RFC PATCH v3 11/19] Implement qmp and hmp commands for notification lists
by Stefan Hajnoczi 23 Oct '12

23 Oct '12

On Fri, Sep 21, 2012 at 01:17:27PM +0200, Vasilis Liaskovitis wrote: > +MemHpInfoList *qmp_query_memory_hotplug(Error **errp) > +{ > + DimmBus *bus = main_memory_bus; > + MemHpInfoList *head = NULL, *cur_item = NULL, *info; > + struct dimm_hp_result *item, *nextitem; > + > + QTAILQ_FOREACH_SAFE(item, &bus->dimm_hp_result_queue, next, nextitem) { > + > + info = g_malloc0(sizeof(*info)); > + info->value = g_malloc0(sizeof(*info->value)); > + info->value->dimm = g_malloc0(sizeof(char) * 32); > + info->value->request = g_malloc0(sizeof(char) * 16); > + info->value->result = g_malloc0(sizeof(char) * 16); > + switch (item->ret) { > + case DIMM_REMOVE_SUCCESS: > + strcpy(info->value->request, "hot-remove"); > + strcpy(info->value->result, "success"); > + break; > + case DIMM_REMOVE_FAIL: > + strcpy(info->value->request, "hot-remove"); > + strcpy(info->value->result, "failure"); > + break; > + case DIMM_ADD_SUCCESS: > + strcpy(info->value->request, "hot-add"); > + strcpy(info->value->result, "success"); > + break; > + case DIMM_ADD_FAIL: > + strcpy(info->value->request, "hot-add"); > + strcpy(info->value->result, "failure"); > + break; > + default: > + break; > + } Any reason to use fixed-size malloc + strcpy() instead of just info->value->X = g_strdup("foo")? Stefan

1 0

Re: [SeaBIOS] [RFC PATCH v3 06/19] Implement "-dimm" command line option
by Avi Kivity 22 Oct '12

22 Oct '12

On 10/19/2012 07:48 PM, Blue Swirl wrote: >>> >>> DIMMs would be allowed to be hotplugged in the generic mem-controller scheme only >>> (unless it makes sense to allow hotplug in the remaining pmc DRBs and >>> start using the generic scheme once we run out of emulated DRBs) >>> >> >> 440fx seems a lost cause, so we can go wild and just implement pv dimms. > > Maybe. But what would be a PV DIMM? Do we need any DIMM-like > granularity at all, instead the guest could be told to use a list of > RAM regions with arbitrary start and end addresses? Guests are likely to support something that has the same constraints as real hardware. If we allow non-power-of-two DIMMs, we might find that guests don't support them well. > Isn't ballooning > also related? It is related in that it is also a memory hotplug technology. But ballooning is subtractive and fine-grained where classic hotplug is additive and coarse grained. We can use both together, but I don't think any work is needed at the qemu level. > >> For q35 I'd like to stay within the spec. > > That may not last forever when machines have terabytes of memory. At least there's work for chipset implementers. Or we can do PV-DIMMs for q35 too. -- error compiling committee.c: too many arguments to function

1 0

Re: [SeaBIOS] [RFC PATCH v3 06/19] Implement "-dimm" command line option
by Avi Kivity 22 Oct '12

22 Oct '12

On 10/18/2012 11:27 AM, Vasilis Liaskovitis wrote: > On Wed, Oct 17, 2012 at 12:03:51PM +0200, Avi Kivity wrote: >> On 10/17/2012 11:19 AM, Vasilis Liaskovitis wrote: >> >> >> >> I don't think so, but probably there's a limit of DIMMs that real >> >> controllers have, something like 8 max. >> > >> > In the case of i440fx specifically, do you mean that we should model the DRB >> > (Dram row boundary registers in section 3.2.19 of the i440fx spec) ? >> > >> > The i440fx DRB registers only supports up to 8 DRAM rows (let's say 1 row >> > maps 1-1 to a DimmDevice for this discussion) and only supports up to 2GB of >> > memory afaict (bit 31 and above is ignored). >> > >> > I 'd rather not model this part of the i440fx - having only 8 DIMMs seems too >> > restrictive. The rest of the patchset supports up to 255 DIMMs so it would be a >> > waste imho to model an old pc memory controller that only supports 8 DIMMs. >> > >> > There was also an old discussion about i440fx modeling here: >> > https://lists.nongnu.org/archive/html/qemu-devel/2011-07/msg02705.html >> > the general direction was that i440fx is too old and we don't want to precisely >> > emulate the DRB registers, since they lack flexibility. >> > >> > Possible solutions: >> > >> > 1) is there a newer and more flexible chipset that we could model? >> >> Look for q35 on this list. > > thanks, I 'll take a look. It sounds like the other options below are more > straightforward now, but let me know if you prefer q35 integration as a priority. At least validate that what you're doing fits with how q35 works. >> >> > >> > We could for example model: >> > - an 8-bit non-cumulative register for each DIMM, denoting how many >> > 128MB chunks it contains. This allowes 32GB for each DIMM, and with 255 DIMMs we >> > describe a bit less than 8TB. These registers require 255 bytes. >> > - a 16-bit cumulative register for each DIMM again for 128MB chunks. This allows >> > us to describe 8TB of memory (but the registers take up double the space, because >> > they describe cumulative memory amounts) >> >> There is no reason to save space. Why not have two 64-bit registers per >> DIMM, one describing the size and the other the base address, both in >> bytes? Use a few low order bits for control. > > Do we want this generic scheme above to be tied into the i440fx/pc machine? Yes. q35 should work according to its own specifications. > Or have it as a separate generic memory bus / pmc usable by others (e.g. in > hw/dimm.c)? > The 64-bit values you describe are already part of DimmDevice properties, but > they are not hardware registers described as part of a chipset. > > In terms of control bits, did you want to mimic some other chipset registers? - > any examples would be useful. I don't have any real requirements. Just make it simple and easily accessible to ACPI code. > >> >> > >> > 3) let everything be handled/abstracted by dimmbus - the chipset DRB modelling >> > is not done (at least for i440fx, other machines could). This is the least precise >> > in terms of emulation. On the other hand, if we are not really trying to emulate >> > the real (too restrictive) hardware, does it matter? >> >> We could emulate base memory using the chipset, and extra memory using >> the scheme above. This allows guests that are tied to the chipset to >> work, and guests that have more awareness (seabios) to use the extra >> features. > > But if we use the real i440fx pmc DRBs for base memory, this means base memory > would be <= 2GB, right? > > Sounds like we 'd need to change the DRBs anyway to describe useful amounts of > base memory (e.g. 512MB chunks and check against address lines [36:29] can > describe base memory up to 64GB, though that's still limiting for very large > VMs). But we'd be diverting from the real hardware again. Then there's no point. Modelling real hardware allows guests written to work against that hardware to function correctly. If you diverge, they won't. > > Then we can model base memory with "tweaked" i440fx pmc's DRB registers - we > could only use DRB[0] (one DIMM describing all of base memory) or more. > > DIMMs would be allowed to be hotplugged in the generic mem-controller scheme only > (unless it makes sense to allow hotplug in the remaining pmc DRBs and > start using the generic scheme once we run out of emulated DRBs) > 440fx seems a lost cause, so we can go wild and just implement pv dimms. For q35 I'd like to stay within the spec. -- error compiling committee.c: too many arguments to function

3 2

[PATCH v3 0/8] q35 seabios support
by Jason Baron 22 Oct '12

22 Oct '12

Hi, Seabios bits for q35 support, I'm posting the qemu changes separately. The patches require '-M pc_q35' and -L 'seabios dir with q35 changes' on the qemu command line. Hopefully, we can make it the default for x86 at some future point when we feel comfortable with it. The current patches have been tested with basic install testing and memory testing on f16, f17, windows 7 and windows 8. They can be run on the various BSD flavors by adding a 'piix4-ide' device to the pci bus. ie: -device piix4-ide. Patches have also been reported to work with a small dsdt change on OSX 10.6 as well. I've dropped the ahci migration bits, which means q35 is not migratable at the moment. I simply haven't had time to make them more complete yet. I'm hoping that we'll come to some agreement on the minimal functionality required for q35 to be merged. Git trees: git://github.com/jibaron/q35-qemu.git git://github.com/jibaron/q35-seabios.git Notes: I've dropped automatic load of the dsdt table on the piix for now. We can't pull this in until we have snapshot of the dsdt aml, and I wanted it to be done at a clean seabios freeze point (Although I guess that could be the current snapshot). I don't see the harm in pulling this in later though. I've also gone to a model of the pci host being sparse: 00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM Controller 00:01.0 VGA compatible controller: Cirrus Logic GD 5446 00:02.0 Ethernet controller: Intel Corporation 82540EM Gigabit Ethernet Controller (rev 03) 00:1f.0 ISA bridge: Intel Corporation 82801IB (ICH9) LPC Interface Controller (rev 02) 00:1f.2 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6 port SATA AHCI Controller (rev 02) 00:1f.3 SMBus: Intel Corporation 82801I (ICH9 Family) SMBus Controller (rev 02) The idea is only to populate the essential stuff at 1f, and have the rest filled out via command line options. In this way we have minimal bus configuration with 1 slot occupied as in piix. Should make things easier for libvirt. And this way user has complete control over things. For example, I have added support that when '-usb' is passed the usb controllers for ich9 are filled out. Todo: -add ahci migration back (need to cover more fields, but basically works) -add base addr for hpet in LPC device (for osx per agraf) -convert hotplug to use MemoryRegionPortio for hotplug (need an IsaDevice?) Thanks, -Jason Changes from v2: -Patch restructure (broke out ich9 chips + data structures separately) -added passthrough support -add support for -usb to fill out host pci bus -Dropped automatic load of dsdt table for piix -cleanups -dropped wmask on smbus (mst) -sparse host bus Changes from v1: -Updated end of low mem from 0xe0000000 -> 0xb0000000 (Gerd Hoffmann) -so 0xb000000-0xc000000 is memconfig -0xc000000-0xfec00000 is 32-bit pci window -style/various cleanups -introduced IF_AHCI -introduced mach_if -split dsdt out of bios, now passed for piix4 as well (Paolo, Gerd) -Removed add opaque argument to pci_map_irq_fn (Michael S. Tsirkin) -removed patches that were merged in v1 Isaku Yamahata (5): seabios: acpi: add mcfg table. seabios: acpi, fadt: make while fadt initialization chipset specific seabios: pci: enable SERR of normal device. seabios: add q35 initialization functions. seabios: q35: add dsdt Jan Kiszka (1): seabios: q35: Register PCI IRQs as active high in APIC mode Jason Baron (2): seabios: make mttr UC area setup dynamic seabios: q35: add basic hotplug support Makefile | 2 +- src/acpi.c | 176 ++++++++- src/acpi.h | 17 + src/config.h | 1 - src/dev-q35.h | 46 +++ src/mtrr.c | 5 +- src/pci.h | 1 + src/pciinit.c | 87 +++++- src/post.c | 6 +- src/q35-acpi-dsdt.dsl | 980 +++++++++++++++++++++++++++++++++++++++++++++++++ src/shadow.c | 13 + src/smm.c | 37 ++ 12 files changed, 1344 insertions(+), 27 deletions(-) create mode 100644 src/dev-q35.h create mode 100644 src/q35-acpi-dsdt.dsl

2 9

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

SeaBIOS