| Message ID | 4EBD5E13.8000604@suse.de |
|---|---|
| State | New |
| Headers | show |
On 11 November 2011 17:40, Alexander Graf <agraf@suse.de> wrote: > Or actually rather this one: > > > Alex > > diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c > index d936809..61b67e8 100644 > --- a/hw/s390-virtio.c > +++ b/hw/s390-virtio.c > @@ -167,6 +167,7 @@ static void s390_init(ram_addr_t my_ram_size, > uint8_t *storage_keys; > void *virtio_region; > target_phys_addr_t virtio_region_len; > + target_phys_addr_t virtio_region_start; > int i; > > /* s390x ram size detection needs a 16bit multiplier + an increment. So > @@ -188,7 +189,9 @@ static void s390_init(ram_addr_t my_ram_size, > > /* clear virtio region */ > virtio_region_len = my_ram_size - ram_size; > - virtio_region = cpu_physical_memory_map(ram_size, &virtio_region_len, > true); > + virtio_region_start = ram_size; > + virtio_region = cpu_physical_memory_map(virtio_region_start, > + &virtio_region_len, true); > memset(virtio_region, 0, virtio_region_len); > cpu_physical_memory_unmap(virtio_region, virtio_region_len, 1, > virtio_region_len); Yes, that looks OK to me. -- PMM
Am 11.11.2011 18:40, schrieb Alexander Graf: > On 11/11/2011 05:44 PM, Peter Maydell wrote: >> On 11 November 2011 16:24, Alexander Graf<agraf@suse.de> wrote: >>> On 11/11/2011 05:11 PM, Peter Maydell wrote: >>>> Yes, so that's option (2) and you need to be using a >>>> target_phys_addr_t. >>> But ram_size is ram_addr_t and is the ram size that I have available >>> to use, >>> so it's exactly the address that I want. I don't see your point. >>> Should go >>> jump through random useless hoops of doing >>> >>> target_phys_addr_t ram_end = ram_size; >>> >>> just because there are some subtile semantic differences between the two >>> variables? They're integers at the end of the day. Both of them. >> Because handing a ram_addr_t to something that wants a target_phys_addr_t >> is a red flag that something might be wrong. (As is handing a variable >> called _size to something that wants an address.) >> >> Force in Newtons and Force in pound-feet are also both just integers >> at the end of the day, but that doesn't make them interchangeable: >> http://en.wikipedia.org/wiki/Mars_Climate_Orbiter#Communications_loss > > Or actually rather this one: > > > Alex > > diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c > index d936809..61b67e8 100644 > --- a/hw/s390-virtio.c > +++ b/hw/s390-virtio.c > @@ -167,6 +167,7 @@ static void s390_init(ram_addr_t my_ram_size, > uint8_t *storage_keys; > void *virtio_region; > target_phys_addr_t virtio_region_len; > + target_phys_addr_t virtio_region_start; > int i; > > /* s390x ram size detection needs a 16bit multiplier + an > increment. So > @@ -188,7 +189,9 @@ static void s390_init(ram_addr_t my_ram_size, > > /* clear virtio region */ > virtio_region_len = my_ram_size - ram_size; > - virtio_region = cpu_physical_memory_map(ram_size, > &virtio_region_len, true); > + virtio_region_start = ram_size; > + virtio_region = cpu_physical_memory_map(virtio_region_start, > + &virtio_region_len, true); Yes, ..._start reads better than ..._size there. Thanks. Andreas
diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c index d936809..61b67e8 100644 --- a/hw/s390-virtio.c +++ b/hw/s390-virtio.c @@ -167,6 +167,7 @@ static void s390_init(ram_addr_t my_ram_size, uint8_t *storage_keys; void *virtio_region; target_phys_addr_t virtio_region_len; + target_phys_addr_t virtio_region_start; int i; /* s390x ram size detection needs a 16bit multiplier + an