[Qemu-devel] Re: [PATCH] QEMU - provide e820 reserve through qemu_cfg

2010-01-26 Thread Jes Sorensen

On 01/26/10 07:46, Gleb Natapov wrote:

On Mon, Jan 25, 2010 at 06:13:35PM +0100, Jes Sorensen wrote:

I am fine with having QEMU build the e820 tables completely if there is
a consensus to take that path.


QEMU can't build the e820 map completely. There are things it doesn't
know. Like how much memory ACPI tables take and where they are located.


Good point!

I think the conclusion is to do a load-extra-tables kinda interface 
allowing QEMU to pass in a bunch of them, but leaving things like the

ACPI space for the BIOS to reserve.

Cheers,
Jes




[Qemu-devel] Re: [PATCH] QEMU - provide e820 reserve through qemu_cfg

2010-01-25 Thread Gleb Natapov
On Mon, Jan 25, 2010 at 06:13:35PM +0100, Jes Sorensen wrote:
> On 01/25/10 17:58, Alexander Graf wrote:
> >Howdy. Congratulations to the new mail address - looks neat ;-).
> 
> :-)
> 
> >Two comments:
> >
> >1) I don't see how passing a single region is any help. I'd rather like to 
> >see a device tree like table structure
> >You'd get one variable for len of the table, one with the contents. So for a 
> >universal reserved region specifier you'd get:
> >
> >
> >
> >Then have len=2 and put data in the table:
> >
> >
> >
> >That way we'd get 2 entries and the chance to enhance them later on. In 
> >fact, it might even make sense to pass the whole table in such a form. That 
> >way qemu generates all of the e820 tables and we can declare whatever we 
> >want. Just add a type field in the table.
> 
> I am fine with having QEMU build the e820 tables completely if there is
> a consensus to take that path.
> 
QEMU can't build the e820 map completely. There are things it doesn't
know. Like how much memory ACPI tables take and where they are located.

> >2) Please inline patches. They showed up as attachments here, making them 
> >really hard to comment on.
> 
> Sorry Thunderbug doesn't do that well, but they should be attached as
> txt?
> 
> Cheers,
> Jes
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Gleb.




[Qemu-devel] Re: [PATCH] QEMU - provide e820 reserve through qemu_cfg

2010-01-25 Thread Jes Sorensen

On 01/25/10 22:08, Alexander Graf wrote:


On 25.01.2010, at 22:05, Jes Sorensen wrote:

Only problem is that we don't really have a way to pass back info
saying 'you messed up trying to pinch an area that the BIOS wants
for itself'.


Eh - the BIOS shouldn't even try to use regions that are declared as reserved 
using this interface.
I guess we're mostly talking about DMI and ACPI tables. They can be anywhere in 
RAM.


What I had in mind with the above was the situation where a user tries
to reserve a region that is hardcoded into the BIOS, such as the address
of the BIOS text/data etc.

I don't think it would be a real problem anyway, if some user wants to
play with it, they have to take the risk of shooting themself in the
foot :)

Cheers,
Jes





[Qemu-devel] Re: [PATCH] QEMU - provide e820 reserve through qemu_cfg

2010-01-25 Thread Jes Sorensen

On 01/25/10 21:14, Anthony Liguori wrote:

On 01/25/2010 02:04 PM, Alexander Graf wrote:

Yes, sounds good. Should be fairly extensible then. What about memory
holes? Do we need to take care of them?


It would be nice for QEMU to be able to add additional e820 regions that
don't necessarily fit the standard layout model.

For instance, I've thought a number of times about using a large
reserved region as a shared memory mechanism.

But we certainly need to allow the BIOS to define the regions it needs
to know about.


I think it should be easy to accommodate using the scheme I am
suggesting. It would require some basic testing for conflicts in the
BIOS, but otherwise it should pretty much allow you to specify any
region you want as a reserved block.

Only problem is that we don't really have a way to pass back info
saying 'you messed up trying to pinch an area that the BIOS wants
for itself'.

I'll take a look at it.

Cheers,
Jes




[Qemu-devel] Re: [PATCH] QEMU - provide e820 reserve through qemu_cfg

2010-01-25 Thread Jes Sorensen

On 01/25/10 18:28, Alexander Graf wrote:

That way we'd get 2 entries and the chance to enhance them later on.
In fact, it might even make sense to pass the whole table in such a
form. That way qemu generates all of the e820 tables and we can
declare whatever we want. Just add a type field in the table.


I am fine with having QEMU build the e820 tables completely if there is
a consensus to take that path.


I agree. We better get this right :-). I don't want to maintain 5
versions of an 380 fw_cfg interface.


Looking at the internals, some of the e820 entries are based on compile
time constants for the BIOS, so it will be hard to pass those from
QEMU, but we could do it in a way so we pass a number of additional
e820 entries. Ie. address, length, and type.

What do you think?

Cheers,
Jes




[Qemu-devel] Re: [PATCH] QEMU - provide e820 reserve through qemu_cfg

2010-01-25 Thread Jes Sorensen

On 01/25/10 17:58, Alexander Graf wrote:

Howdy. Congratulations to the new mail address - looks neat ;-).


:-)


Two comments:

1) I don't see how passing a single region is any help. I'd rather like to see 
a device tree like table structure
You'd get one variable for len of the table, one with the contents. So for a 
universal reserved region specifier you'd get:



Then have len=2 and put data in the table:



That way we'd get 2 entries and the chance to enhance them later on. In fact, 
it might even make sense to pass the whole table in such a form. That way qemu 
generates all of the e820 tables and we can declare whatever we want. Just add 
a type field in the table.


I am fine with having QEMU build the e820 tables completely if there is
a consensus to take that path.


2) Please inline patches. They showed up as attachments here, making them 
really hard to comment on.


Sorry Thunderbug doesn't do that well, but they should be attached as
txt?

Cheers,
Jes





[Qemu-devel] Re: [PATCH] QEMU - provide e820 reserve through qemu_cfg

2010-01-25 Thread Alexander Graf

On 25.01.2010, at 22:05, Jes Sorensen wrote:

> On 01/25/10 21:14, Anthony Liguori wrote:
>> On 01/25/2010 02:04 PM, Alexander Graf wrote:
>>> Yes, sounds good. Should be fairly extensible then. What about memory
>>> holes? Do we need to take care of them?
>> 
>> It would be nice for QEMU to be able to add additional e820 regions that
>> don't necessarily fit the standard layout model.
>> 
>> For instance, I've thought a number of times about using a large
>> reserved region as a shared memory mechanism.
>> 
>> But we certainly need to allow the BIOS to define the regions it needs
>> to know about.
> 
> I think it should be easy to accommodate using the scheme I am
> suggesting. It would require some basic testing for conflicts in the
> BIOS, but otherwise it should pretty much allow you to specify any
> region you want as a reserved block.
> 
> Only problem is that we don't really have a way to pass back info
> saying 'you messed up trying to pinch an area that the BIOS wants
> for itself'.

Eh - the BIOS shouldn't even try to use regions that are declared as reserved 
using this interface.
I guess we're mostly talking about DMI and ACPI tables. They can be anywhere in 
RAM.

Alex



[Qemu-devel] Re: [PATCH] QEMU - provide e820 reserve through qemu_cfg

2010-01-25 Thread Anthony Liguori

On 01/25/2010 02:04 PM, Alexander Graf wrote:

On 25.01.2010, at 18:46, Jes Sorensen wrote:

   

On 01/25/10 18:28, Alexander Graf wrote:
 

That way we'd get 2 entries and the chance to enhance them later on.
In fact, it might even make sense to pass the whole table in such a
form. That way qemu generates all of the e820 tables and we can
declare whatever we want. Just add a type field in the table.
   

I am fine with having QEMU build the e820 tables completely if there is
a consensus to take that path.
 

I agree. We better get this right :-). I don't want to maintain 5
versions of an 380 fw_cfg interface.
   

Looking at the internals, some of the e820 entries are based on compile
time constants for the BIOS, so it will be hard to pass those from
QEMU, but we could do it in a way so we pass a number of additional
e820 entries. Ie. address, length, and type.
 

Yes, sounds good. Should be fairly extensible then. What about memory holes? Do 
we need to take care of them?
   


It would be nice for QEMU to be able to add additional e820 regions that 
don't necessarily fit the standard layout model.


For instance, I've thought a number of times about using a large 
reserved region as a shared memory mechanism.


But we certainly need to allow the BIOS to define the regions it needs 
to know about.


Regards,

Anthony Liguori




[Qemu-devel] Re: [PATCH] QEMU - provide e820 reserve through qemu_cfg

2010-01-25 Thread Alexander Graf

On 25.01.2010, at 18:46, Jes Sorensen wrote:

> On 01/25/10 18:28, Alexander Graf wrote:
 That way we'd get 2 entries and the chance to enhance them later on.
 In fact, it might even make sense to pass the whole table in such a
 form. That way qemu generates all of the e820 tables and we can
 declare whatever we want. Just add a type field in the table.
>>> 
>>> I am fine with having QEMU build the e820 tables completely if there is
>>> a consensus to take that path.
>> 
>> I agree. We better get this right :-). I don't want to maintain 5
>> versions of an 380 fw_cfg interface.
> 
> Looking at the internals, some of the e820 entries are based on compile
> time constants for the BIOS, so it will be hard to pass those from
> QEMU, but we could do it in a way so we pass a number of additional
> e820 entries. Ie. address, length, and type.

Yes, sounds good. Should be fairly extensible then. What about memory holes? Do 
we need to take care of them?

Alex



[Qemu-devel] Re: [PATCH] QEMU - provide e820 reserve through qemu_cfg

2010-01-25 Thread Alexander Graf


Am 25.01.2010 um 18:13 schrieb Jes Sorensen :


On 01/25/10 17:58, Alexander Graf wrote:

Howdy. Congratulations to the new mail address - looks neat ;-).


:-)


Two comments:

1) I don't see how passing a single region is any help. I'd rather  
like to see a device tree like table structure
You'd get one variable for len of the table, one with the contents.  
So for a universal reserved region specifier you'd get:




Then have len=2 and put data in the table:



That way we'd get 2 entries and the chance to enhance them later  
on. In fact, it might even make sense to pass the whole table in  
such a form. That way qemu generates all of the e820 tables and we  
can declare whatever we want. Just add a type field in the table.


I am fine with having QEMU build the e820 tables completely if there  
is

a consensus to take that path.


I agree. We better get this right :-). I don't want to maintain 5  
versions of an 380 fw_cfg interface.




2) Please inline patches. They showed up as attachments here,  
making them really hard to comment on.


Sorry Thunderbug doesn't do that well, but they should be attached as
txt?


I suppose so, but Apple Mail still doesn't show it and definitely  
doesn't let me comment on it.


The easiest way for me so far has been to use git-send-email and a  
bash alias to fill in login information.



Alex







[Qemu-devel] Re: [PATCH] QEMU - provide e820 reserve through qemu_cfg

2010-01-25 Thread Alexander Graf

On 25.01.2010, at 17:52, Jes Sorensen wrote:

> Hi,
> 
> This is the QEMU patch for providing the e820-reserve space through
> qemu-cfg.

Howdy. Congratulations to the new mail address - looks neat ;-).


Two comments:

1) I don't see how passing a single region is any help. I'd rather like to see 
a device tree like table structure
You'd get one variable for len of the table, one with the contents. So for a 
universal reserved region specifier you'd get:



Then have len=2 and put data in the table:



That way we'd get 2 entries and the chance to enhance them later on. In fact, 
it might even make sense to pass the whole table in such a form. That way qemu 
generates all of the e820 tables and we can declare whatever we want. Just add 
a type field in the table.

2) Please inline patches. They showed up as attachments here, making them 
really hard to comment on.


Alex