Patchwork [10/17] s390x: keep hint on virtio managing size

login
register
mail settings
Submitter Alexander Graf
Date April 15, 2011, 3:32 p.m.
Message ID <1302881578-5357-11-git-send-email-agraf@suse.de>
Download mbox | patch
Permalink /patch/91409/
State New
Headers show

Comments

Alexander Graf - April 15, 2011, 3:32 p.m.
The s390x virtio bus keeps management information on virtio after the top
of the guest's RAM. We need to be able to tell the guest the size of its
RAM (without virtio stuff), but also be able to trap when the guest accesses
RAM outside of its scope (including virtio stuff).

So we need a variable telling us the size of the virtio stuff, so we can
calculate the highest available RAM address from that.

While at it, also increase the maximum number of virtio pages, so we play
along well with more recent kernels that spawn a ridiculous number of virtio
console adapters.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/s390-virtio-bus.c |    3 +++
 hw/s390-virtio-bus.h |    2 +-
 target-s390x/cpu.h   |    3 +++
 3 files changed, 7 insertions(+), 1 deletions(-)
Aurelien Jarno - April 18, 2011, 7:06 p.m.
On Fri, Apr 15, 2011 at 05:32:51PM +0200, Alexander Graf wrote:
> The s390x virtio bus keeps management information on virtio after the top
> of the guest's RAM. We need to be able to tell the guest the size of its
> RAM (without virtio stuff), but also be able to trap when the guest accesses
> RAM outside of its scope (including virtio stuff).
> 
> So we need a variable telling us the size of the virtio stuff, so we can
> calculate the highest available RAM address from that.
> 
> While at it, also increase the maximum number of virtio pages, so we play
> along well with more recent kernels that spawn a ridiculous number of virtio
> console adapters.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  hw/s390-virtio-bus.c |    3 +++
>  hw/s390-virtio-bus.h |    2 +-
>  target-s390x/cpu.h   |    3 +++
>  3 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
> index bb49e39..a90963b 100644
> --- a/hw/s390-virtio-bus.c
> +++ b/hw/s390-virtio-bus.c
> @@ -60,6 +60,9 @@ static const VirtIOBindings virtio_s390_bindings;
>  
>  static ram_addr_t s390_virtio_device_num_vq(VirtIOS390Device *dev);
>  
> +/* length of VirtIO device pages */
> +target_phys_addr_t virtio_size = S390_DEVICE_PAGES * TARGET_PAGE_SIZE;
> +

If this variable is never written, can it be declared const?

>  VirtIOS390Bus *s390_virtio_bus_init(ram_addr_t *ram_size)
>  {
>      VirtIOS390Bus *bus;
> diff --git a/hw/s390-virtio-bus.h b/hw/s390-virtio-bus.h
> index edf6d04..0c412d0 100644
> --- a/hw/s390-virtio-bus.h
> +++ b/hw/s390-virtio-bus.h
> @@ -33,7 +33,7 @@
>  #define VIRTIO_VQCONFIG_LEN		24
>  
>  #define VIRTIO_RING_LEN			(TARGET_PAGE_SIZE * 3)
> -#define S390_DEVICE_PAGES		256
> +#define S390_DEVICE_PAGES		512
>  
>  typedef struct VirtIOS390Device {
>      DeviceState qdev;
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index a84b3ee..6ba303d 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -293,6 +293,9 @@ void kvm_s390_interrupt_internal(CPUState *env, int type, uint32_t parm,
>                                   uint64_t parm64, int vm);
>  CPUState *s390_cpu_addr2state(uint16_t cpu_addr);
>  
> +/* from s390-virtio-bus */
> +extern target_phys_addr_t virtio_size;
> +
>  #ifndef KVM_S390_SIGP_STOP
>  #define KVM_S390_SIGP_STOP              0
>  #define KVM_S390_PROGRAM_INT            0
Alexander Graf - April 18, 2011, 9:03 p.m.
On 18.04.2011, at 21:06, Aurelien Jarno wrote:

> On Fri, Apr 15, 2011 at 05:32:51PM +0200, Alexander Graf wrote:
>> The s390x virtio bus keeps management information on virtio after the top
>> of the guest's RAM. We need to be able to tell the guest the size of its
>> RAM (without virtio stuff), but also be able to trap when the guest accesses
>> RAM outside of its scope (including virtio stuff).
>> 
>> So we need a variable telling us the size of the virtio stuff, so we can
>> calculate the highest available RAM address from that.
>> 
>> While at it, also increase the maximum number of virtio pages, so we play
>> along well with more recent kernels that spawn a ridiculous number of virtio
>> console adapters.
>> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> hw/s390-virtio-bus.c |    3 +++
>> hw/s390-virtio-bus.h |    2 +-
>> target-s390x/cpu.h   |    3 +++
>> 3 files changed, 7 insertions(+), 1 deletions(-)
>> 
>> diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
>> index bb49e39..a90963b 100644
>> --- a/hw/s390-virtio-bus.c
>> +++ b/hw/s390-virtio-bus.c
>> @@ -60,6 +60,9 @@ static const VirtIOBindings virtio_s390_bindings;
>> 
>> static ram_addr_t s390_virtio_device_num_vq(VirtIOS390Device *dev);
>> 
>> +/* length of VirtIO device pages */
>> +target_phys_addr_t virtio_size = S390_DEVICE_PAGES * TARGET_PAGE_SIZE;
>> +
> 
> If this variable is never written, can it be declared const?

Does that improve anything for exported variables? Do they get put into a ro section then?


Alex
Aurelien Jarno - April 20, 2011, 10:21 a.m.
On Mon, Apr 18, 2011 at 11:03:59PM +0200, Alexander Graf wrote:
> 
> On 18.04.2011, at 21:06, Aurelien Jarno wrote:
> 
> > On Fri, Apr 15, 2011 at 05:32:51PM +0200, Alexander Graf wrote:
> >> The s390x virtio bus keeps management information on virtio after the top
> >> of the guest's RAM. We need to be able to tell the guest the size of its
> >> RAM (without virtio stuff), but also be able to trap when the guest accesses
> >> RAM outside of its scope (including virtio stuff).
> >> 
> >> So we need a variable telling us the size of the virtio stuff, so we can
> >> calculate the highest available RAM address from that.
> >> 
> >> While at it, also increase the maximum number of virtio pages, so we play
> >> along well with more recent kernels that spawn a ridiculous number of virtio
> >> console adapters.
> >> 
> >> Signed-off-by: Alexander Graf <agraf@suse.de>
> >> ---
> >> hw/s390-virtio-bus.c |    3 +++
> >> hw/s390-virtio-bus.h |    2 +-
> >> target-s390x/cpu.h   |    3 +++
> >> 3 files changed, 7 insertions(+), 1 deletions(-)
> >> 
> >> diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
> >> index bb49e39..a90963b 100644
> >> --- a/hw/s390-virtio-bus.c
> >> +++ b/hw/s390-virtio-bus.c
> >> @@ -60,6 +60,9 @@ static const VirtIOBindings virtio_s390_bindings;
> >> 
> >> static ram_addr_t s390_virtio_device_num_vq(VirtIOS390Device *dev);
> >> 
> >> +/* length of VirtIO device pages */
> >> +target_phys_addr_t virtio_size = S390_DEVICE_PAGES * TARGET_PAGE_SIZE;
> >> +
> > 
> > If this variable is never written, can it be declared const?
> 
> Does that improve anything for exported variables? Do they get put into a ro section then?
> 

Unfortunately, that will still be an exported variable, and it's a pitty
to use that just because we can't have a #define in hw/* included in
target-*/* . That said I just realized that a lot of target are actually
doing that, maybe it's the solution here.

Adding a const here will clearly show to people reading the code that
it's nothing more than advanced #define.

Patch

diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
index bb49e39..a90963b 100644
--- a/hw/s390-virtio-bus.c
+++ b/hw/s390-virtio-bus.c
@@ -60,6 +60,9 @@  static const VirtIOBindings virtio_s390_bindings;
 
 static ram_addr_t s390_virtio_device_num_vq(VirtIOS390Device *dev);
 
+/* length of VirtIO device pages */
+target_phys_addr_t virtio_size = S390_DEVICE_PAGES * TARGET_PAGE_SIZE;
+
 VirtIOS390Bus *s390_virtio_bus_init(ram_addr_t *ram_size)
 {
     VirtIOS390Bus *bus;
diff --git a/hw/s390-virtio-bus.h b/hw/s390-virtio-bus.h
index edf6d04..0c412d0 100644
--- a/hw/s390-virtio-bus.h
+++ b/hw/s390-virtio-bus.h
@@ -33,7 +33,7 @@ 
 #define VIRTIO_VQCONFIG_LEN		24
 
 #define VIRTIO_RING_LEN			(TARGET_PAGE_SIZE * 3)
-#define S390_DEVICE_PAGES		256
+#define S390_DEVICE_PAGES		512
 
 typedef struct VirtIOS390Device {
     DeviceState qdev;
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index a84b3ee..6ba303d 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -293,6 +293,9 @@  void kvm_s390_interrupt_internal(CPUState *env, int type, uint32_t parm,
                                  uint64_t parm64, int vm);
 CPUState *s390_cpu_addr2state(uint16_t cpu_addr);
 
+/* from s390-virtio-bus */
+extern target_phys_addr_t virtio_size;
+
 #ifndef KVM_S390_SIGP_STOP
 #define KVM_S390_SIGP_STOP              0
 #define KVM_S390_PROGRAM_INT            0