Re: [SeaBIOS] [PATCH] vgabios: implement AX=1120H..1124H functions
On Thu, Jan 10, 2013 at 01:41:36PM +0100, Paolo Bonzini wrote: > These function only have to set INT 1Fh and INT 43h, and set > the BDA height + number of rows. > > I could not find out whether AX=1120h should also set the character > height to 8. I think not, because INT 43h might still point to > 14- or 16-pixel high characters and in this case INT 1Fh will not > be used at all. Thanks. Can you give a brief description on what these functions do and why it's useful to have them implemented? -Kevin ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC PATCH v4 00/30] ACPI memory hotplug
> > > > IIRC q35 supports memory hotplug natively (picked up in some > > discussion). Is that correct? > > > From previous discussion I also understand that q35 supports native hotplug. > Sections 5.1 and 5.2 of the spec describe the MCH registers but the native > memory hotplug specifics are not yet clear to me. Any pointers from the > spec are welcome. Ping. Could anyone who's familiar with the q35 spec provide some pointers on native memory hotplug details in the spec? I see pcie hotplug registers but can't find memory hotplug interface details. If I am not mistaken, the spec is here: http://www.intel.com/design/chipsets/datashts/316966.htm Is the q35 memory hotplug support supposed to be an shpc-like interface geared towards memory slots instead of pci slots? thanks, - Vasilis ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [RFC PATCH v4 00/30] ACPI memory hotplug
Am 10.01.2013 18:36, schrieb Vasilis Liaskovitis: > Btw, is the CPU link feature already implemented in a qom-cpu branch? No, the latest topology series moves fields around as preparation. There's still one CPUState per hyperthread, but as a DeviceState now. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC PATCH v4 21/30] Implement dimm-info
On Tue, Jan 08, 2013 at 04:20:26PM -0700, Eric Blake wrote:
> On 12/18/2012 05:41 AM, Vasilis Liaskovitis wrote:
> > "query-dimm-info" and "info dimm" will give current state of all dimms in
> > the
> > system e.g.
> >
> > dimm0: on
> > dimm1: off
> > dimm2: off
> > dimm3: on
> > etc.
> >
> > Signed-off-by: Vasilis Liaskovitis
> > ---
>
> > +++ b/qapi-schema.json
> > @@ -2914,6 +2914,32 @@
> > { 'command': 'query-memory-total', 'returns': 'int' }
> >
> > ##
> > +# @DimmInfo:
> > +#
> > +# Information about status of a memory hotplug command
> > +#
> > +# @dimm: the Dimm associated with the result
> > +#
> > +# @result: the result of the hotplug command
>
> Here you call it 'result',
>
> > +#
> > +# Since: 1.4
> > +#
> > +##
> > +{ 'type': 'DimmInfo',
> > + 'data': {'dimm': 'str', 'state': 'bool'} }
>
> but here you call it 'state'. Which is it? And does 'true' mean
> plugged in, or that the last command succeeded (where the last command
> may have been either a plug or an unplug)? My preference is that 'true'
> means plugged in, so more documentation would help.
"True" does mean "plugged in" as you suggest, and the name should be "state". I
'll clarify the documentation.
>
> > +
> > +##
> > +# @query-dimm-info:
> > +#
> > +# Returns total memory in bytes, including hotplugged dimms
>
> Really?
>
> > +#
> > +# Returns: int
>
> Copy-and-paste error? This doesn't return an 'int', but an array of
> 'DimmInfo'.
both copy-paste errors, will fix.
thanks,
- Vasilis
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [RFC PATCH v4 19/30] Implement "info memory-total" and "query-memory-total"
On Fri, Jan 04, 2013 at 09:21:08AM -0700, Eric Blake wrote:
> On 12/18/2012 05:41 AM, Vasilis Liaskovitis wrote:
> > Returns total physical memory available to guest in bytes, including
> > hotplugged
> > memory. Note that the number reported here may be different from what the
> > guest
> > sees e.g. if the guest has not logically onlined hotplugged memory.
> >
> > This functionality is provided independently of a balloon device, since a
> > guest can be using ACPI memory hotplug without using a balloon device.
> >
> > v3->v4: Moved qmp command implementation to vl.c. This prevents a circular
> > header dependency problem.
>
> Generally, patch change history should occur...
>
> >
> > Signed-off-by: Vasilis Liaskovitis
> > ---
>
> ...here, after the --- divider. It's useful in the email chain, but
> does not need to be part of the final git history.
ok, thanks.
>
> > +++ b/qapi-schema.json
> > @@ -2903,6 +2903,17 @@
> > { 'command': 'query-target', 'returns': 'TargetInfo' }
> >
> > ##
> > +# @query-memory-total:
> > +#
> > +# Returns total memory in bytes, including hotplugged dimms
> > +#
> > +# Returns: int
> > +#
> > +# Since: 1.4
> > +##
> > +{ 'command': 'query-memory-total', 'returns': 'int' }
>
> Any reason you can't name this just 'query-memory', and return a JSON
> dictionary instead of a single int, so that in the future you can add
> other memory parameters into the same call? For example, down the road
> we may want to report some 'newstat' without adding a new QMP command:
I am fine with a dictionary, if we see a need for extending the command in the
future. Is it common practice to start off with dicts for simple commands? In
any case, I 'll update.
thanks,
- Vasilis
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [Qemu-devel] [RFC PATCH v4 00/30] ACPI memory hotplug
Hi, On Wed, Jan 09, 2013 at 01:08:52AM +0100, Andreas Färber wrote: > Am 18.12.2012 13:41, schrieb Vasilis Liaskovitis: > > Because dimm layout needs to be configured on machine-boot, all dimm devices > > need to be specified on startup command line (either with populated=on or > > with > > populated=off). The dimm information is stored in dimm configuration > > structures. > > > > After machine startup, dimms are hot-added or removed with normal device_add > > and device_del operations e.g.: > > Hot-add syntax: "device_add dimm,id=mydimm0,bus=membus.0" > > Hot-remove syntax: "device_del dimm,id=mydimm0" > > This sounds contradictory: Either all devices need to be specified on > the command line, or they can be hot-added via monitor. Due to the fixed layout requirement, all memory devices need to be specified at the command line. This was done with a separate "-dimm" argument in previous versions (see v3), but some reviewers didn't like the extra argument and suggested handling everything with the normal "-device" arg. So "-device dimm,..." saves the layout for *all* memory devices. However for "populated=off" dimms, the device is actually *not* created at startup. This is why the following combination is not contradictory: Dimm descirption at startup: -device dimm,id=mydimm0,bus=membus.0,size=1G,node=0,populated=off Hot-add with monitor command: device_add dimm,id=mydimm0,bus=membus.0 If on the other hand we specify: -device dimm,id=mydimm0,bus=membus.0,size=1G,node=0,populated=on the dimm device is indeed created at startup. granted it's confusing, but this is how v4 handles the fixed layout/device creation without adding a new command line argument for the layout. Better solutions are welcome. > > Assuming a fixed layout at startup, I wonder if there is another clever > way to model this... For CPU hotplug Anthony had suggested to have a > fixed set of link properties that get set to a CPU socket as > needed. Might a similar strategy work for memory, i.e. a > startup-configured amount of links on /machine/dimm[n] that point > to a QOM DIMM object or NULL if unpopulated? Hot(un)plug would then > simply work via QMP qom-set command. (CC'ing some people) This may work for a fixed number of PV dimms. On the other hand some other reviewers like the idea of modelling the memory bus (DimmBus), either for paravirtualized features (e.g. i440fx) or for emulated memory controllers in the future. I assume we either go with a bus or links<>, and not both. Btw, is the CPU link feature already implemented in a qom-cpu branch? I haven't tested qom-cpu for a long time, but I could take a look as a point of reference. thanks, - Vasilis ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] Get rid of a compile time dprintf warning
Thanks for looking into this. I'm using gcc version 4.6.3 > > > > I'm building this in cygwin, and using our (Sage's) toolchain. > > What version of gcc is it? > > If I change the format to %ld I get: > > Compile checking out/post.o > src/post.c: In function ‘reloc_init’: > src/post.c:350:5: warning: format ‘%ld’ expects argument of type > ‘long int’, but argument 4 has type ‘int’ [-Wformat] > > Which is what I'd expect. Maybe there is something odd with your > version of gcc? > > $ gcc --version > gcc (GCC) 4.7.2 20120921 (Red Hat 4.7.2-2) > > and I get similar behavior with gcc v3.4. > > -Kevin > ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
[SeaBIOS] [PATCH] vgabios: implement AX=1120H..1124H functions
These function only have to set INT 1Fh and INT 43h, and set
the BDA height + number of rows.
I could not find out whether AX=1120h should also set the character
height to 8. I think not, because INT 43h might still point to
14- or 16-pixel high characters and in this case INT 1Fh will not
be used at all.
Signed-off-by: Paolo Bonzini
---
vgasrc/vgabios.c | 60
1 file changed, 60 insertions(+)
diff --git a/vgasrc/vgabios.c b/vgasrc/vgabios.c
index b13d274..afaf018 100644
--- a/vgasrc/vgabios.c
+++ b/vgasrc/vgabios.c
@@ -797,6 +797,61 @@ handle_101114(struct bregs *regs)
}
static void
+handle_101120(struct bregs *regs)
+{
+SET_IVT(0x1f, SEGOFF(regs->es, regs->bp));
+}
+
+void
+load_gfx_font(u16 seg, u16 off, u8 height, u8 bl, u8 dl)
+{
+u8 rows;
+
+SET_IVT(0x43, SEGOFF(seg, off));
+switch(bl) {
+case 0:
+rows = dl;
+break;
+case 1:
+rows = 14;
+break;
+case 3:
+rows = 43;
+break;
+case 2:
+default:
+rows = 25;
+break;
+}
+SET_BDA(video_rows, rows - 1);
+SET_BDA(char_height, height);
+}
+
+static void
+handle_101121(struct bregs *regs)
+{
+load_gfx_font(regs->es, regs->bp, regs->cx, regs->bl, regs->dl);
+}
+
+static void
+handle_101122(struct bregs *regs)
+{
+load_gfx_font(get_global_seg(), (u32)vgafont14, 14, regs->bl, regs->dl);
+}
+
+static void
+handle_101123(struct bregs *regs)
+{
+load_gfx_font(get_global_seg(), (u32)vgafont8, 8, regs->bl, regs->dl);
+}
+
+static void
+handle_101124(struct bregs *regs)
+{
+load_gfx_font(get_global_seg(), (u32)vgafont16, 16, regs->bl, regs->dl);
+}
+
+static void
handle_101130(struct bregs *regs)
{
switch (regs->bh) {
@@ -867,6 +922,11 @@ handle_1011(struct bregs *regs)
case 0x12: handle_101112(regs); break;
case 0x14: handle_101114(regs); break;
case 0x30: handle_101130(regs); break;
+case 0x20: handle_101120(regs); break;
+case 0x21: handle_101121(regs); break;
+case 0x22: handle_101122(regs); break;
+case 0x23: handle_101123(regs); break;
+case 0x24: handle_101124(regs); break;
default: handle_1011XX(regs); break;
}
}
--
1.8.1
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] Get rid of a compile time dprintf warning
On 01/10/13 00:54, Kevin O'Connor wrote:
> On Wed, Jan 09, 2013 at 08:34:18AM -0600, Dave Frodin wrote:
>> Here's a patch that's been lingering awhile.
>> thanks,
>
> Thanks. I don't receive a warning for this - what is the exact
> warning you receive? I don't see why gcc would convert (datalow_end -
> datalow_start) to a long.
In the expression "datalow_end - datalow_start", both operands
- (have incomplete array type (size unknown), ISO C99 6.2.5p22),
- are converted ("decay") to type "pointer-to-u8" (ISO C99 6.3.2.1p3).
The expression "datalow_end - datalow_start" invokes undefined behavior,
because the (decayed) operands are not pointers into the same array (or
to the element one past the last element in the array).
Anyway, the result type of "datalow_end - datalow_start" would be
ptrdiff_t, whose size is implementation-defined.
>From ISO C99, 6.5.6 Additive operators (normative):
9 When two pointers are subtracted, both shall point to elements of
the same array object, or one past the last element of the array
object; the result is the difference of the subscripts of the two
array elements. The size of the result is implementation-defined,
and its type (a signed integer type) is ptrdiff_t defined in the
header. If the result is not representable in an object
of that type, the behavior is undefined. In other words, if the
expressions P and Q point to, respectively, the i-th and j-th
elements of an array object, the expression (P)-(Q) has the value
i-j provided the value fits in an object of type ptrdiff_t.
Moreover, if the expression P points either to an element of an
array object or one past the last element of an array object, and
the expression Q points to the last element of the same array
object, the expression ((Q)+1)-(P) has the same value as ((Q)-(P))+1
and as -((P)-((Q)+1)), and has the value zero if the expression P
points one past the last element of the array object, even though
the expression (Q)+1 does not point to an element of the array
object.88)
Footnote 88 (informative)
Another way to approach pointer arithmetic is first to convert the
pointer(s) to character pointer(s): In this scheme the integer
expression added to or subtracted from the converted pointer is
first multiplied by the size of the object originally pointed to,
and the resulting pointer is converted back to the original type.
For pointer subtraction, the result of the difference between the
character pointers is similarly divided by the size of the object
originally pointed to.
When viewed in this way, an implementation need only provide one
extra byte (which may overlap another object in the program) just
after the end of the object in order to satisfy the "one past the
last element" requirements.
I can see two ways to solve this "problem" (many are possible probably):
(1) print the difference (of type ptrdiff_t) with the "%td" printf()
conversion specification. It was first defined in SUSv3
,
ie. not standard C. However this leaves the undefined behavior (the
subtraction) in place.
(2) Convert the operands first to pointer-to-void (safe), then to
uintptr_t ((a) an optional type that is required on XSI conformant
systems, (b) the conversion is safe from void*), then take their
difference, convert it to uintmax_t, and print it with "%"PRIuMAX:
dprintf(1, "Relocating low data from %p to %p (size %"PRIuMAX")\n"
, (void *)datalow_start, (void *)final_datalow_start,
, (uintmax_t)( (uintptr_t)(void *)datalow_end
- (uintptr_t)(void *)datalow_start));
(I'm of course aware that you won't do this, bit I think it explains the
"problem" and you could simplify from here, perhaps exploiting
characteristics that are guaranteed for any platform that runs SeaBIOS.)
Laszlo
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
