October 2012 - SeaBIOS - mail.coreboot.org

Re: [SeaBIOS] Git over http
by Kevin O'Connor Oct. 28, 2012

Oct. 28, 2012

On Mon, Oct 15, 2012 at 05:34:26PM +0200, Christian Gmeiner wrote: > Hi all > > At the moment seabios is only available via > git clone git://git.seabios.org/seabios.git seabios > > It would be really cool if it would be also accessible via > git clone http://git.seabios.org/seabios.git seabios > > For build-servers etc. behind a firewall. > > thanks There is a mirror available via: git clone http://review.coreboot.org/p/seabios.git -Kevin

1 0

Re: [SeaBIOS] [Qemu-devel] [RFC PATCH v3 05/19] Implement dimm device abstraction
by liu ping fan Oct. 25, 2012

Oct. 25, 2012

On Thu, Oct 25, 2012 at 1:16 AM, Vasilis Liaskovitis <vasilis.liaskovitis(a)profitbricks.com> wrote: > 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? > My original meaning is that flush out the dangling pointer. >> >> 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. > For current code, I think to block on the listener to wait for the completion of flushing out. But after mr->ops's ref/unref patchset accept, we can release the ref of RAM device after we have done with it (it is a very raw idea, need to improve). > 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 > Yes. > 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. > Yes. > 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. > The guest is one of the users of dimm device, and block layer is another one. Regards, pingfan >> Synchronous cancel behavior is not workable since it can lead to poor >> latency or hangs in the guest. > > ok > > thanks, > > - Vasilis >

1 0

[PATCH 4/4] bochsvga: validate EDID resolution in init
by Hiroshi Miura Oct. 25, 2012

Oct. 25, 2012

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 Oct. 25, 2012

Oct. 25, 2012

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 Oct. 25, 2012

Oct. 25, 2012

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 Oct. 24, 2012

Oct. 24, 2012

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 Oct. 24, 2012

Oct. 24, 2012

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 Oct. 24, 2012

Oct. 24, 2012

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 Oct. 23, 2012

Oct. 23, 2012

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 Oct. 22, 2012

Oct. 22, 2012

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