Re: [Qemu-devel] [PATCH v3 0/7] Let boards state maximum RAM limits in QEMUMachine struct
On 04/04/11 19:26, Peter Maydell wrote: On 4 April 2011 15:53, Jes Sorensen jes.soren...@redhat.com wrote: I understand that what you are proposing seems to work well enough for your problem at hand. What I am saying is that adding a mechanism like that, can cause problems for adding a more generic mechanism that handles more advanced boards in the future. I much prefer a generic solution than a simple hack. I don't think it's a hack. I think it's a reasonably clean solution to the set of cases we've actually encountered in practice, and I think trying to design something more generalised without actually having a use case to tie it to is just going to produce something complicated which doesn't turn out to actually be what a hypothetical advanced board will actually need. I think we're much better off with code that does what we need it to do now, and designing and implementing the complicated generic framework only when we actually need it. Sorry but it is not a clean solution at all, it's a simple hack based on a linear model, which happens to work in practice when dealing with some simple boards. I suggested a very simple interface that would work fine for embedded boards, and which would be simple to implement. The so called advanced boards you're referring to are out there in numbers already, you probably have a couple of them. They are called PCs. As I pointed out before, this is not a theoretical problem, most numa systems have this issue, including many x86 boxes. I can see the problem also existing with mips boards like the sb1250 ones I worked on many years ago. OK, so presumably you can provide a qemu command line and an image which demonstrates the problem... Look at the -numa argument and show me the code that actually validates the memory amount correctly in that case? Jes
Re: [Qemu-devel] [PATCH v3 0/7] Let boards state maximum RAM limits in QEMUMachine struct
On 04/04/11 18:54, Blue Swirl wrote: On Mon, Apr 4, 2011 at 5:53 PM, Jes Sorensen jes.soren...@redhat.com wrote: I understand that what you are proposing seems to work well enough for your problem at hand. What I am saying is that adding a mechanism like that, can cause problems for adding a more generic mechanism that handles more advanced boards in the future. I much prefer a generic solution than a simple hack. We could have -device RAM,base=xxx,size=yyy,id=DIMM1 -numa nodeid=zzz,memory=DIMM1 for fine tuned control. But asking users to list and bind the DIMMs needed just to have some amount of RAM is a bit too much. So we also need a simple case (-m) and a simple check for the max memory. I totally agree, but the suggestion I proposed earlier doesn't in any way prevent this. If we use a table of valid memory locations for a given board, then it is really easy for each board to provide a validation function which accepts the amount or rejects it. As I pointed out before, this is not a theoretical problem, most numa systems have this issue, including many x86 boxes. I can see the problem also existing with mips boards like the sb1250 ones I worked on many years ago. Having an a table of valid ram locations for a board, will also give you a framework to validate against if you want to be able to specify chunks of memory at different areas of a board. This could be useful for testing behavior that is like it would be if you have a system where installing different DIMMs would split the RAM up differently. Maybe we could remove some of memory logic in pc.c with this approach. I believe it would simplify things a great deal, and have the benefit that we can emulate things much more realistically. The only issue is that it takes a little more work up front, but it really isn't a big deal. Cheers, Jes
Re: [Qemu-devel] [PATCH v3 0/7] Let boards state maximum RAM limits in QEMUMachine struct
On 03/30/11 16:07, Peter Maydell wrote: On 30 March 2011 14:56, Anthony Liguori anth...@codemonkey.ws wrote: On 03/30/2011 08:22 AM, Peter Maydell wrote: Not really, typically they're just filled up in some particular order (main RAM in one place and expansion RAM elsewhere). Since the board init function is only passed a single ram_size parameter that's all you can do anyhow. FWIW, I don't think any static data is going to be perfect here. A lot of boards have strict requirements on ram_size based on plausible combinations of DIMMs. Arbitrary values up to ram_size is not good enough. ram_size ought to be viewed as a hint, not a mechanism to allow common code to completely validate the passed in ram size parameter. It's still up to the board to validate that the given ram size makes sense. Yes, I agree, so we shouldn't try to specify some complicated set of static data that still won't be good enough. I'm trying to make it easy for boards to avoid crashing horribly when the user passes a bad value; that's all. If you don't validate properly, is there really a point in introducing that value anyway? From what you write, it sounds like it can still fail for some limits of the memory valid if the config is wrong? It still seems to me it would be better to have the boards present a table of valid memory ranges so we can do a proper validation of the valud? Cheers, Jes
Re: [Qemu-devel] [PATCH v3 0/7] Let boards state maximum RAM limits in QEMUMachine struct
On 04/04/11 16:42, Peter Maydell wrote: On 4 April 2011 15:29, Jes Sorensen jes.soren...@redhat.com wrote: Yes, I agree, so we shouldn't try to specify some complicated set of static data that still won't be good enough. I'm trying to make it easy for boards to avoid crashing horribly when the user passes a bad value; that's all. If you don't validate properly, is there really a point in introducing that value anyway? From what you write, it sounds like it can still fail for some limits of the memory valid if the config is wrong? For the boards I care about (the ARM ones), the only validation requirement is that we don't allow the user to specify so much ram that we overlap physical RAM with I/O space. So ram_size is good enough. For the sun4m boards we can assume that the only validation they need is a ram_size check, because that's all they do at the moment and nobody's complaining that I know of. I understand that what you are proposing seems to work well enough for your problem at hand. What I am saying is that adding a mechanism like that, can cause problems for adding a more generic mechanism that handles more advanced boards in the future. I much prefer a generic solution than a simple hack. It still seems to me it would be better to have the boards present a table of valid memory ranges so we can do a proper validation of the valud? If you have a concrete example of multiple boards which we currently model and which require this level of flexibility to avoid odd misbehaviour trying to run a guest, then please point them out and I'll look at expanding the patch to cover their requirements. If this is just a theoretical issue, then I think we should only add the extra generic framework code if and when we turn out to need it. As I pointed out before, this is not a theoretical problem, most numa systems have this issue, including many x86 boxes. I can see the problem also existing with mips boards like the sb1250 ones I worked on many years ago. Having an a table of valid ram locations for a board, will also give you a framework to validate against if you want to be able to specify chunks of memory at different areas of a board. This could be useful for testing behavior that is like it would be if you have a system where installing different DIMMs would split the RAM up differently. Jes
Re: [Qemu-devel] [PATCH v3 0/7] Let boards state maximum RAM limits in QEMUMachine struct
On 4 April 2011 15:29, Jes Sorensen jes.soren...@redhat.com wrote: On 03/30/11 16:07, Peter Maydell wrote: On 30 March 2011 14:56, Anthony Liguori anth...@codemonkey.ws wrote: On 03/30/2011 08:22 AM, Peter Maydell wrote: Not really, typically they're just filled up in some particular order (main RAM in one place and expansion RAM elsewhere). Since the board init function is only passed a single ram_size parameter that's all you can do anyhow. FWIW, I don't think any static data is going to be perfect here. A lot of boards have strict requirements on ram_size based on plausible combinations of DIMMs. Arbitrary values up to ram_size is not good enough. ram_size ought to be viewed as a hint, not a mechanism to allow common code to completely validate the passed in ram size parameter. It's still up to the board to validate that the given ram size makes sense. Yes, I agree, so we shouldn't try to specify some complicated set of static data that still won't be good enough. I'm trying to make it easy for boards to avoid crashing horribly when the user passes a bad value; that's all. If you don't validate properly, is there really a point in introducing that value anyway? From what you write, it sounds like it can still fail for some limits of the memory valid if the config is wrong? For the boards I care about (the ARM ones), the only validation requirement is that we don't allow the user to specify so much ram that we overlap physical RAM with I/O space. So ram_size is good enough. For the sun4m boards we can assume that the only validation they need is a ram_size check, because that's all they do at the moment and nobody's complaining that I know of. As far as I know Anthony is suggesting that some boards might in theory have stricter memory size requirements. I agree that this is a possibility, but if you have a rare board which has an idiosyncratic requirement then the correct way to handle that is to add extra checks in the board init functions. I don't see a huge queue of people with patches to add complex checks to board init functions that would indicate that we need a generic mechanism for handling this. What we do have is simple ram_size limit checks (in sun4m and we need them for the arm devboards) -- which is a demonstrated need that justifies the common code framework. It still seems to me it would be better to have the boards present a table of valid memory ranges so we can do a proper validation of the valud? If you have a concrete example of multiple boards which we currently model and which require this level of flexibility to avoid odd misbehaviour trying to run a guest, then please point them out and I'll look at expanding the patch to cover their requirements. If this is just a theoretical issue, then I think we should only add the extra generic framework code if and when we turn out to need it. -- PMM
Re: [Qemu-devel] [PATCH v3 0/7] Let boards state maximum RAM limits in QEMUMachine struct
On Mon, Apr 4, 2011 at 5:53 PM, Jes Sorensen jes.soren...@redhat.com wrote: On 04/04/11 16:42, Peter Maydell wrote: On 4 April 2011 15:29, Jes Sorensen jes.soren...@redhat.com wrote: Yes, I agree, so we shouldn't try to specify some complicated set of static data that still won't be good enough. I'm trying to make it easy for boards to avoid crashing horribly when the user passes a bad value; that's all. If you don't validate properly, is there really a point in introducing that value anyway? From what you write, it sounds like it can still fail for some limits of the memory valid if the config is wrong? For the boards I care about (the ARM ones), the only validation requirement is that we don't allow the user to specify so much ram that we overlap physical RAM with I/O space. So ram_size is good enough. For the sun4m boards we can assume that the only validation they need is a ram_size check, because that's all they do at the moment and nobody's complaining that I know of. I understand that what you are proposing seems to work well enough for your problem at hand. What I am saying is that adding a mechanism like that, can cause problems for adding a more generic mechanism that handles more advanced boards in the future. I much prefer a generic solution than a simple hack. We could have -device RAM,base=xxx,size=yyy,id=DIMM1 -numa nodeid=zzz,memory=DIMM1 for fine tuned control. But asking users to list and bind the DIMMs needed just to have some amount of RAM is a bit too much. So we also need a simple case (-m) and a simple check for the max memory. It's still possible for the board to do additional checks and even use heuristics to invent the DIMMs. It still seems to me it would be better to have the boards present a table of valid memory ranges so we can do a proper validation of the valud? If you have a concrete example of multiple boards which we currently model and which require this level of flexibility to avoid odd misbehaviour trying to run a guest, then please point them out and I'll look at expanding the patch to cover their requirements. If this is just a theoretical issue, then I think we should only add the extra generic framework code if and when we turn out to need it. As I pointed out before, this is not a theoretical problem, most numa systems have this issue, including many x86 boxes. I can see the problem also existing with mips boards like the sb1250 ones I worked on many years ago. Having an a table of valid ram locations for a board, will also give you a framework to validate against if you want to be able to specify chunks of memory at different areas of a board. This could be useful for testing behavior that is like it would be if you have a system where installing different DIMMs would split the RAM up differently. Maybe we could remove some of memory logic in pc.c with this approach. By the way, fw_cfg only uses one number for the memory size. This should be changed. Alternatively the BIOSes could do a more realistic RAM probe, but that would be wasteful.
Re: [Qemu-devel] [PATCH v3 0/7] Let boards state maximum RAM limits in QEMUMachine struct
On 4 April 2011 15:53, Jes Sorensen jes.soren...@redhat.com wrote: I understand that what you are proposing seems to work well enough for your problem at hand. What I am saying is that adding a mechanism like that, can cause problems for adding a more generic mechanism that handles more advanced boards in the future. I much prefer a generic solution than a simple hack. I don't think it's a hack. I think it's a reasonably clean solution to the set of cases we've actually encountered in practice, and I think trying to design something more generalised without actually having a use case to tie it to is just going to produce something complicated which doesn't turn out to actually be what a hypothetical advanced board will actually need. I think we're much better off with code that does what we need it to do now, and designing and implementing the complicated generic framework only when we actually need it. If you have a concrete example of multiple boards which we currently model and which require this level of flexibility to avoid odd misbehaviour trying to run a guest, then please point them out and I'll look at expanding the patch to cover their requirements. If this is just a theoretical issue, then I think we should only add the extra generic framework code if and when we turn out to need it. As I pointed out before, this is not a theoretical problem, most numa systems have this issue, including many x86 boxes. I can see the problem also existing with mips boards like the sb1250 ones I worked on many years ago. OK, so presumably you can provide a qemu command line and an image which demonstrates the problem... Having an a table of valid ram locations for a board ...I'm not sure this is even meaningful for boards where you can remap the RAM to different physical addresses at runtime (versatile express ought to do this although we don't currently model that bit). -- PMM
Re: [Qemu-devel] [PATCH v3 0/7] Let boards state maximum RAM limits in QEMUMachine struct
On 03/29/11 16:08, Peter Maydell wrote: This primary aim of this patchset is to add a new 'max_ram' field to the QEMUMachine structure so that a board model can specify the maximum RAM it will accept. We can then produce a friendly diagnostic message when the user tries to start qemu with a '-m' option asking for more RAM than that. (Currently most of the ARM devboard models respond with an obscure guest crash when the guest tries to access RAM and finds device registers instead.) If no maximum size is specified we default to the old behaviour of do not impose any limit. The bulk of the patchset is knock-on cleanup as a result, in particular allowing QEMUMachine structs to be const and sun4m cleanup. Hi Peter, Sorry for not getting to this patch earlier. I am a little concerned about this approach. It should work for simple embedded boards, but for larger systems, it really ought to be a mask rather than a max address. On NUMA systems you are bound to have holes, and at times you might not even start from address 0. In these cases you are likely to have device registers mapped in between the memory chunks. Ideally I think it would be better to have a mask and then introduce a is_valid_memory() kinda function to check it with. I fear that by introducing a simple max limit like this, we are going to hit problems later when we try to improve the NUMA support. Cheers, Jes
Re: [Qemu-devel] [PATCH v3 0/7] Let boards state maximum RAM limits in QEMUMachine struct
On 30 March 2011 08:48, Jes Sorensen jes.soren...@redhat.com wrote: On 03/29/11 16:08, Peter Maydell wrote: This primary aim of this patchset is to add a new 'max_ram' field to the QEMUMachine structure so that a board model can specify the maximum RAM it will accept. I am a little concerned about this approach. It should work for simple embedded boards, but for larger systems, it really ought to be a mask rather than a max address. It's not a maximum address, it's a maximum size. For instance the RAM isn't contiguous on some of the ARM devboards. Ideally I think it would be better to have a mask and then introduce a is_valid_memory() kinda function to check it with. The command line option doesn't provide any means of saying put 64MB in this hole and another 128 over here and 32 there, so this seems completely pointless to me. All we are trying to do is validate what the user has asked for, so why have a validation mechanism that can cope with impossible-to-request arrangements? I fear that by introducing a simple max limit like this, we are going to hit problems later when we try to improve the NUMA support. I think this is letting the best be the enemy of the good. Even if you do want to have NUMA systems do more complex things I think you should still have the simple maximum size approach for the bulk of the supported boards which don't need anything more complicated. So additional NUMA features would augment, not replace this. -- PMM
Re: [Qemu-devel] [PATCH v3 0/7] Let boards state maximum RAM limits in QEMUMachine struct
On 03/30/11 10:09, Peter Maydell wrote: On 30 March 2011 08:48, Jes Sorensen jes.soren...@redhat.com wrote: I am a little concerned about this approach. It should work for simple embedded boards, but for larger systems, it really ought to be a mask rather than a max address. It's not a maximum address, it's a maximum size. For instance the RAM isn't contiguous on some of the ARM devboards. Right, but the fact that you can have holes makes it even more an issue. Ideally I think it would be better to have a mask and then introduce a is_valid_memory() kinda function to check it with. The command line option doesn't provide any means of saying put 64MB in this hole and another 128 over here and 32 there, so this seems completely pointless to me. All we are trying to do is validate what the user has asked for, so why have a validation mechanism that can cope with impossible-to-request arrangements? You can on x86 using the -numa argument. When you use that, it is completely pointless to have the max memory limit, to use your own words. I fear that by introducing a simple max limit like this, we are going to hit problems later when we try to improve the NUMA support. I think this is letting the best be the enemy of the good. Even if you do want to have NUMA systems do more complex things I think you should still have the simple maximum size approach for the bulk of the supported boards which don't need anything more complicated. So additional NUMA features would augment, not replace this. The problem is we end up with two mechanisms that way which won't necessarily live well together. Since you have holes on ARM too, it makes sense IMHO to use a mask exactly because you can then map that to max memory by simply adding up the available holes. A linear number on the other hand is much harder to validate against once you start populating holes explicitly. Cheers, Jes
Re: [Qemu-devel] [PATCH v3 0/7] Let boards state maximum RAM limits in QEMUMachine struct
On 30 March 2011 11:51, Jes Sorensen jes.soren...@redhat.com wrote: On 03/30/11 10:09, Peter Maydell wrote: On 30 March 2011 08:48, Jes Sorensen jes.soren...@redhat.com wrote: I am a little concerned about this approach. It should work for simple embedded boards, but for larger systems, it really ought to be a mask rather than a max address. It's not a maximum address, it's a maximum size. For instance the RAM isn't contiguous on some of the ARM devboards. Right, but the fact that you can have holes makes it even more an issue. Not really, typically they're just filled up in some particular order (main RAM in one place and expansion RAM elsewhere). Since the board init function is only passed a single ram_size parameter that's all you can do anyhow. Ideally I think it would be better to have a mask and then introduce a is_valid_memory() kinda function to check it with. I'm not sure what this mask would look like. You want megabyte granularity, but even if you assume only 48 bit physical addresses that would still be an alarmingly large number of bits. So I guess you'd actually want a list of (start,length) tuples. I strongly dislike this approach. I think it introduces unneeded extra complexity, and it doesn't even address all the cases you might want a generic validate RAM options mechanism to do (eg boards which only allow RAM in 16MB increments, boards where you must fill the base RAM first and then the expansion RAM, boards where the RAM isn't at a specific physical address but can be remapped under guest control, just to pick three). So it falls awkwardly between the two stools of flexible and generic and simple and straightforward. The command line option doesn't provide any means of saying put 64MB in this hole and another 128 over here and 32 there, You can on x86 using the -numa argument. When you use that, it is completely pointless to have the max memory limit, to use your own words. (Is there any documentation on what you can do with the -numa option? http://qemu.weilnetz.de/qemu-doc.html is decidedly laconic.) Since you have holes on ARM too, it makes sense IMHO to use a mask exactly because you can then map that to max memory by simply adding up the available holes. A linear number on the other hand is much harder to validate against once you start populating holes explicitly. But I don't want to populate holes explicitly, and I don't imagine most of the other boards want to populate holes explicitly either. The current QEMU design basically only controls the total amount of RAM (as a command line option, and as a parameter to the board init function), with NUMA being a an optional extra. Non-NUMA is the common use case and you want the API used by the board code to be straightforward to implement this common case. Adding a simple maximum RAM field fits in with the existing design and solves a genuine requirement for multiple board models. Even if one system happens to have NUMA-specific requirements we don't want to have to have every single devboard specify its RAM limits in a complicated fashion for the benefit of the one NUMA system. Just have the NUMA system not specify max_ram (so it gets the default of zero, ie don't check) and have some NUMA-specific QEMUMachine fields which default to system doesn't support NUMA. Then NUMA aware board models can do the right thing, and non-NUMA boards also do the right thing without having to think about NUMA-related fields at all. (Compare the way non-SMP boards don't have to worry about the max_cpus field.) -- PMM
Re: [Qemu-devel] [PATCH v3 0/7] Let boards state maximum RAM limits in QEMUMachine struct
On 03/30/2011 08:22 AM, Peter Maydell wrote: On 30 March 2011 11:51, Jes Sorensenjes.soren...@redhat.com wrote: On 03/30/11 10:09, Peter Maydell wrote: On 30 March 2011 08:48, Jes Sorensenjes.soren...@redhat.com wrote: I am a little concerned about this approach. It should work for simple embedded boards, but for larger systems, it really ought to be a mask rather than a max address. It's not a maximum address, it's a maximum size. For instance the RAM isn't contiguous on some of the ARM devboards. Right, but the fact that you can have holes makes it even more an issue. Not really, typically they're just filled up in some particular order (main RAM in one place and expansion RAM elsewhere). Since the board init function is only passed a single ram_size parameter that's all you can do anyhow. FWIW, I don't think any static data is going to be perfect here. A lot of boards have strict requirements on ram_size based on plausible combinations of DIMMs. Arbitrary values up to ram_size is not good enough. ram_size ought to be viewed as a hint, not a mechanism to allow common code to completely validate the passed in ram size parameter. It's still up to the board to validate that the given ram size makes sense. Regards, Anthony Liguori Ideally I think it would be better to have a mask and then introduce a is_valid_memory() kinda function to check it with. I'm not sure what this mask would look like. You want megabyte granularity, but even if you assume only 48 bit physical addresses that would still be an alarmingly large number of bits. So I guess you'd actually want a list of (start,length) tuples. I strongly dislike this approach. I think it introduces unneeded extra complexity, and it doesn't even address all the cases you might want a generic validate RAM options mechanism to do (eg boards which only allow RAM in 16MB increments, boards where you must fill the base RAM first and then the expansion RAM, boards where the RAM isn't at a specific physical address but can be remapped under guest control, just to pick three). So it falls awkwardly between the two stools of flexible and generic and simple and straightforward. The command line option doesn't provide any means of saying put 64MB in this hole and another 128 over here and 32 there, You can on x86 using the -numa argument. When you use that, it is completely pointless to have the max memory limit, to use your own words. (Is there any documentation on what you can do with the -numa option? http://qemu.weilnetz.de/qemu-doc.html is decidedly laconic.) Since you have holes on ARM too, it makes sense IMHO to use a mask exactly because you can then map that to max memory by simply adding up the available holes. A linear number on the other hand is much harder to validate against once you start populating holes explicitly. But I don't want to populate holes explicitly, and I don't imagine most of the other boards want to populate holes explicitly either. The current QEMU design basically only controls the total amount of RAM (as a command line option, and as a parameter to the board init function), with NUMA being a an optional extra. Non-NUMA is the common use case and you want the API used by the board code to be straightforward to implement this common case. Adding a simple maximum RAM field fits in with the existing design and solves a genuine requirement for multiple board models. Even if one system happens to have NUMA-specific requirements we don't want to have to have every single devboard specify its RAM limits in a complicated fashion for the benefit of the one NUMA system. Just have the NUMA system not specify max_ram (so it gets the default of zero, ie don't check) and have some NUMA-specific QEMUMachine fields which default to system doesn't support NUMA. Then NUMA aware board models can do the right thing, and non-NUMA boards also do the right thing without having to think about NUMA-related fields at all. (Compare the way non-SMP boards don't have to worry about the max_cpus field.) -- PMM
Re: [Qemu-devel] [PATCH v3 0/7] Let boards state maximum RAM limits in QEMUMachine struct
On 03/30/11 15:22, Peter Maydell wrote:
On 30 March 2011 11:51, Jes Sorensen jes.soren...@redhat.com
wrote:
Ideally I think it would be better to have a mask and then
introduce a is_valid_memory() kinda function to check it with.
I'm not sure what this mask would look like. You want megabyte
granularity, but even if you assume only 48 bit physical addresses
that would still be an alarmingly large number of bits. So I guess
you'd actually want a list of (start,length) tuples.
I strongly dislike this approach. I think it introduces unneeded
extra complexity, and it doesn't even address all the cases you might
want a generic validate RAM options mechanism to do (eg boards
which only allow RAM in 16MB increments, boards where you must fill
the base RAM first and then the expansion RAM, boards where the RAM
isn't at a specific physical address but can be remapped under guest
control, just to pick three). So it falls awkwardly between the two
stools of flexible and generic and simple and straightforward.
I thought about it a bit and obviously a simple mask isn't going to cut
it. What we should have is more like a list of valid ram locations which
we can use to calculate the allowable size for. The -m parameter is
really a shortcut for the real thing, and we shouldn't optimize for -m,
but rather the other way round.
You can on x86 using the -numa argument. When you use that, it is
completely pointless to have the max memory limit, to use your own
words.
(Is there any documentation on what you can do with the -numa option?
http://qemu.weilnetz.de/qemu-doc.html is decidedly laconic.)
I don't know of documentation for that unfortunately :( Eventually you
should be able to do anything with it that can happen in a real numa
system. Basically it should allow you to define nodes and assign CPUs
and memory to the given nodes, as well as PCI devices.
Since you have holes on ARM too, it makes sense IMHO to use a mask
exactly because you can then map that to max memory by simply
adding up the available holes. A linear number on the other hand is
much harder to validate against once you start populating holes
explicitly.
But I don't want to populate holes explicitly, and I don't imagine
most of the other boards want to populate holes explicitly either.
Why not? You just mentioned the case where board holes can be populated
in different ways?
The current QEMU design basically only controls the total amount of
RAM (as a command line option, and as a parameter to the board init
function), with NUMA being a an optional extra. Non-NUMA is the
common use case and you want the API used by the board code to be
straightforward to implement this common case. Adding a simple
maximum RAM field fits in with the existing design and solves a
genuine requirement for multiple board models.
The problem with the existing design is exactly that it was designed for
the simple case, and works badly for the case that matches the hardware
better. I suggest we do it right, and then provide functions to make -m
work easily. Otherwise we end up with bad hacks that are going to work
badly for the non simple case.
Even if one system happens to have NUMA-specific requirements we
don't want to have to have every single devboard specify its RAM
limits in a complicated fashion for the benefit of the one NUMA
system. Just have the NUMA system not specify max_ram (so it gets the
default of zero, ie don't check) and have some NUMA-specific
QEMUMachine fields which default to system doesn't support NUMA.
Then NUMA aware board models can do the right thing, and non-NUMA
boards also do the right thing without having to think about
NUMA-related fields at all. (Compare the way non-SMP boards don't
have to worry about the max_cpus field.)
A simple system is basically a single-node NUMA system. So you refer to
NODE 0 per default if no explicit node is listed. Instead what we should
have is something like this
struct node_memregion {
uint64_t start0, end0;
uint64_t start1, end1;
uint64_t start2, end2;
uint64_t start3, end3;
}
Then have a max-nodes value per machine, and a pointer to a
node_memregion array. If you only have one node, which would be the
common case, then you populate just that specifying the holes you have
etc. A simple function can then calculate the maximum allowable memory
in a generic way.
Simple to define for all the simple boards.
Jes
Re: [Qemu-devel] [PATCH v3 0/7] Let boards state maximum RAM limits in QEMUMachine struct
On 30 March 2011 14:56, Anthony Liguori anth...@codemonkey.ws wrote: On 03/30/2011 08:22 AM, Peter Maydell wrote: Not really, typically they're just filled up in some particular order (main RAM in one place and expansion RAM elsewhere). Since the board init function is only passed a single ram_size parameter that's all you can do anyhow. FWIW, I don't think any static data is going to be perfect here. A lot of boards have strict requirements on ram_size based on plausible combinations of DIMMs. Arbitrary values up to ram_size is not good enough. ram_size ought to be viewed as a hint, not a mechanism to allow common code to completely validate the passed in ram size parameter. It's still up to the board to validate that the given ram size makes sense. Yes, I agree, so we shouldn't try to specify some complicated set of static data that still won't be good enough. I'm trying to make it easy for boards to avoid crashing horribly when the user passes a bad value; that's all. -- PMM
[Qemu-devel] [PATCH v3 0/7] Let boards state maximum RAM limits in QEMUMachine struct
This primary aim of this patchset is to add a new 'max_ram' field to the QEMUMachine structure so that a board model can specify the maximum RAM it will accept. We can then produce a friendly diagnostic message when the user tries to start qemu with a '-m' option asking for more RAM than that. (Currently most of the ARM devboard models respond with an obscure guest crash when the guest tries to access RAM and finds device registers instead.) If no maximum size is specified we default to the old behaviour of do not impose any limit. The bulk of the patchset is knock-on cleanup as a result, in particular allowing QEMUMachine structs to be const and sun4m cleanup. Changes in v3: * as suggested by Blue Swirl, new patch 3 to make qemu_register_machine take a const QEMUMachine * rather than a non-const one * this makes the sun4m patch (old 3, new 4) simpler as we don't have to move 'const' qualifiers around * new patch 7 which adds 'const' to all the board QEMUMachine definitions Changes in v2: * use target_physaddr_t rather than ram_addr_t for max_ram, so we can specify maximum ram sizes for 64 bit target boards * new patches 3,4 which update sun4m to use the generic max_ram, so we can delete the sun4m-specific code which was doing the same job * patch 5 does some tidy-up of sun4m init functions; not strictly related but the assert() at least is enabled by the cleanup done in patch 3. Peter Maydell (7): Allow boards to specify maximum RAM size hw: Add maximum RAM specifications for ARM devboard models vl.c: Fix machine registration so QEMUMachine structs can be const hw/sun4m: Move QEMUMachine structs into sun4*_hwdef structs hw/sun4m: Use the QEMUMachine max_ram to implement memory limit hw/sun4m: Use a macro to hide the repetitive board init functions hw: Make QEMUMachine structure definitions const hw/an5206.c |2 +- hw/axis_dev88.c |2 +- hw/boards.h |6 +- hw/dummy_m68k.c |2 +- hw/etraxfs.c |2 +- hw/gumstix.c |4 +- hw/integratorcp.c |3 +- hw/leon3.c|2 +- hw/lm32_boards.c |4 +- hw/mainstone.c|2 +- hw/mcf5208.c |2 +- hw/mips_fulong2e.c|2 +- hw/mips_jazz.c|4 +- hw/mips_malta.c |2 +- hw/mips_mipssim.c |2 +- hw/mips_r4k.c |2 +- hw/musicpal.c |2 +- hw/nseries.c |4 +- hw/omap_sx1.c |4 +- hw/palm.c |2 +- hw/pc_piix.c | 12 +- hw/petalogix_ml605_mmu.c |2 +- hw/petalogix_s3adsp1800_mmu.c |2 +- hw/ppc405_boards.c|4 +- hw/ppc440_bamboo.c|4 +- hw/ppc_newworld.c |2 +- hw/ppc_oldworld.c |2 +- hw/ppc_prep.c |2 +- hw/ppce500_mpc8544ds.c|2 +- hw/r2d.c |2 +- hw/realview.c | 19 ++- hw/s390-virtio.c |2 +- hw/shix.c |2 +- hw/spitz.c|8 +- hw/stellaris.c|4 +- hw/sun4m.c| 523 - hw/sun4u.c|6 +- hw/syborg.c |2 +- hw/tosa.c |2 +- hw/versatilepb.c |9 +- hw/virtex_ml507.c |2 +- hw/xen_machine_pv.c |2 +- vl.c | 115 ++ 43 files changed, 363 insertions(+), 422 deletions(-)
