jcs.org / qemu
qemu with hax to log dma reads & writes jcs.org/2018/11/12/vfio
fork atom

arm/aspeed: actually check RAM size

It's supposed that SOC will check if "-m" provided
RAM size is valid by setting "ram-size" property and
then board would read back valid (possibly corrected
value) to map RAM MemoryReging with valid size.
It isn't doing so, since check is called only
indirectly from
aspeed_sdmc_reset()->asc->compute_conf()
or much later when guest writes to configuration
register.

So depending on "-m" value QEMU end-ups with a warning
and an invalid MemoryRegion size allocated and mapped.
(examples:
-M ast2500-evb -m 1M
0000000080000000-000000017ffffffe (prio 0, i/o): aspeed-ram-container
0000000080000000-00000000800fffff (prio 0, ram): ram
0000000080100000-00000000bfffffff (prio 0, i/o): max_ram
-M ast2500-evb -m 3G
0000000080000000-000000017ffffffe (prio 0, i/o): aspeed-ram-container
0000000080000000-000000013fffffff (prio 0, ram): ram
[DETECTED OVERFLOW!] 0000000140000000-00000000bfffffff (prio 0, i/o): max_ram
)
On top of that sdmc falls back and reports to guest
"default" size, it thinks machine should have.

This patch makes ram-size check actually work and
changes behavior from a warning later on during
machine reset to error_fatal at the moment SOC.ram-size
is set so user will have to fix RAM size on CLI
to start machine.

It also gets out of the way mutable ram-size logic,
so we could consolidate RAM allocation logic around
pre-allocated hostmem backend (supplied by user or
auto created by generic machine code depending on
supplied -m/mem-path/mem-prealloc options.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20200219160953.13771-10-imammedo@redhat.com>

authored by

Igor Mammedov and committed by
Patchew Importer 533eb415 b844d822

+70 -27
+5 -8
hw/arm/aspeed.c
··· 204 204 205 205 sc = ASPEED_SOC_GET_CLASS(&bmc->soc); 206 206 207 + /* 208 + * This will error out if isize is not supported by memory controller. 209 + */ 207 210 object_property_set_uint(OBJECT(&bmc->soc), ram_size, "ram-size", 208 - &error_abort); 211 + &error_fatal); 212 + 209 213 object_property_set_int(OBJECT(&bmc->soc), amc->hw_strap1, "hw-strap1", 210 214 &error_abort); 211 215 object_property_set_int(OBJECT(&bmc->soc), amc->hw_strap2, "hw-strap2", ··· 227 231 } 228 232 object_property_set_bool(OBJECT(&bmc->soc), true, "realized", 229 233 &error_abort); 230 - 231 - /* 232 - * Allocate RAM after the memory controller has checked the size 233 - * was valid. If not, a default value is used. 234 - */ 235 - ram_size = object_property_get_uint(OBJECT(&bmc->soc), "ram-size", 236 - &error_abort); 237 234 238 235 memory_region_allocate_system_memory(&bmc->ram, NULL, "ram", ram_size); 239 236 memory_region_add_subregion(&bmc->ram_container, 0, &bmc->ram);
+64 -19
hw/misc/aspeed_sdmc.c
··· 17 17 #include "migration/vmstate.h" 18 18 #include "qapi/error.h" 19 19 #include "trace.h" 20 + #include "qemu/units.h" 21 + #include "qemu/cutils.h" 22 + #include "qapi/visitor.h" 20 23 21 24 /* Protection Key Register */ 22 25 #define R_PROT (0x00 / 4) ··· 160 163 case 512: 161 164 return ASPEED_SDMC_DRAM_512MB; 162 165 default: 166 + g_assert_not_reached(); 163 167 break; 164 168 } 165 - 166 - /* use a common default */ 167 - warn_report("Invalid RAM size 0x%" PRIx64 ". Using default 256M", 168 - s->ram_size); 169 - s->ram_size = 256 << 20; 170 - return ASPEED_SDMC_DRAM_256MB; 171 169 } 172 170 173 171 static int ast2500_rambits(AspeedSDMCState *s) ··· 182 180 case 1024: 183 181 return ASPEED_SDMC_AST2500_1024MB; 184 182 default: 183 + g_assert_not_reached(); 185 184 break; 186 185 } 187 - 188 - /* use a common default */ 189 - warn_report("Invalid RAM size 0x%" PRIx64 ". Using default 512M", 190 - s->ram_size); 191 - s->ram_size = 512 << 20; 192 - return ASPEED_SDMC_AST2500_512MB; 193 186 } 194 187 195 188 static int ast2600_rambits(AspeedSDMCState *s) ··· 204 197 case 2048: 205 198 return ASPEED_SDMC_AST2600_2048MB; 206 199 default: 200 + g_assert_not_reached(); 207 201 break; 208 202 } 209 - 210 - /* use a common default */ 211 - warn_report("Invalid RAM size 0x%" PRIx64 ". Using default 1024M", 212 - s->ram_size); 213 - s->ram_size = 1024 << 20; 214 - return ASPEED_SDMC_AST2600_1024MB; 215 203 } 216 204 217 205 static void aspeed_sdmc_reset(DeviceState *dev) ··· 225 213 s->regs[R_CONF] = asc->compute_conf(s, 0); 226 214 } 227 215 216 + static void aspeed_sdmc_get_ram_size(Object *obj, Visitor *v, const char *name, 217 + void *opaque, Error **errp) 218 + { 219 + AspeedSDMCState *s = ASPEED_SDMC(obj); 220 + int64_t value = s->ram_size; 221 + 222 + visit_type_int(v, name, &value, errp); 223 + } 224 + 225 + static void aspeed_sdmc_set_ram_size(Object *obj, Visitor *v, const char *name, 226 + void *opaque, Error **errp) 227 + { 228 + int i; 229 + char *sz; 230 + int64_t value; 231 + Error *local_err = NULL; 232 + AspeedSDMCState *s = ASPEED_SDMC(obj); 233 + AspeedSDMCClass *asc = ASPEED_SDMC_GET_CLASS(s); 234 + 235 + visit_type_int(v, name, &value, &local_err); 236 + if (local_err) { 237 + error_propagate(errp, local_err); 238 + return; 239 + } 240 + 241 + for (i = 0; asc->valid_ram_sizes[i]; i++) { 242 + if (value == asc->valid_ram_sizes[i]) { 243 + s->ram_size = value; 244 + return; 245 + } 246 + } 247 + 248 + sz = size_to_str(value); 249 + error_setg(&local_err, "Invalid RAM size %s", sz); 250 + g_free(sz); 251 + error_propagate(errp, local_err); 252 + } 253 + 254 + static void aspeed_sdmc_initfn(Object *obj) 255 + { 256 + object_property_add(obj, "ram-size", "int", 257 + aspeed_sdmc_get_ram_size, aspeed_sdmc_set_ram_size, 258 + NULL, NULL, NULL); 259 + } 260 + 228 261 static void aspeed_sdmc_realize(DeviceState *dev, Error **errp) 229 262 { 230 263 SysBusDevice *sbd = SYS_BUS_DEVICE(dev); ··· 249 282 }; 250 283 251 284 static Property aspeed_sdmc_properties[] = { 252 - DEFINE_PROP_UINT64("ram-size", AspeedSDMCState, ram_size, 0), 253 285 DEFINE_PROP_UINT64("max-ram-size", AspeedSDMCState, max_ram_size, 0), 254 286 DEFINE_PROP_END_OF_LIST(), 255 287 }; ··· 268 300 .name = TYPE_ASPEED_SDMC, 269 301 .parent = TYPE_SYS_BUS_DEVICE, 270 302 .instance_size = sizeof(AspeedSDMCState), 303 + .instance_init = aspeed_sdmc_initfn, 271 304 .class_init = aspeed_sdmc_class_init, 272 305 .class_size = sizeof(AspeedSDMCClass), 273 306 .abstract = true, ··· 298 331 s->regs[reg] = data; 299 332 } 300 333 334 + static const uint64_t 335 + aspeed_2400_ram_sizes[] = { 64 * MiB, 128 * MiB, 256 * MiB, 512 * MiB, 0}; 336 + 301 337 static void aspeed_2400_sdmc_class_init(ObjectClass *klass, void *data) 302 338 { 303 339 DeviceClass *dc = DEVICE_CLASS(klass); ··· 307 343 asc->max_ram_size = 512 << 20; 308 344 asc->compute_conf = aspeed_2400_sdmc_compute_conf; 309 345 asc->write = aspeed_2400_sdmc_write; 346 + asc->valid_ram_sizes = aspeed_2400_ram_sizes; 310 347 } 311 348 312 349 static const TypeInfo aspeed_2400_sdmc_info = { ··· 350 387 351 388 s->regs[reg] = data; 352 389 } 390 + 391 + static const uint64_t 392 + aspeed_2500_ram_sizes[] = { 128 * MiB, 256 * MiB, 512 * MiB, 1024 * MiB, 0}; 353 393 354 394 static void aspeed_2500_sdmc_class_init(ObjectClass *klass, void *data) 355 395 { ··· 360 400 asc->max_ram_size = 1024 << 20; 361 401 asc->compute_conf = aspeed_2500_sdmc_compute_conf; 362 402 asc->write = aspeed_2500_sdmc_write; 403 + asc->valid_ram_sizes = aspeed_2500_ram_sizes; 363 404 } 364 405 365 406 static const TypeInfo aspeed_2500_sdmc_info = { ··· 404 445 s->regs[reg] = data; 405 446 } 406 447 448 + static const uint64_t 449 + aspeed_2600_ram_sizes[] = { 256 * MiB, 512 * MiB, 1024 * MiB, 2048 * MiB, 0}; 450 + 407 451 static void aspeed_2600_sdmc_class_init(ObjectClass *klass, void *data) 408 452 { 409 453 DeviceClass *dc = DEVICE_CLASS(klass); ··· 413 457 asc->max_ram_size = 2048 << 20; 414 458 asc->compute_conf = aspeed_2600_sdmc_compute_conf; 415 459 asc->write = aspeed_2600_sdmc_write; 460 + asc->valid_ram_sizes = aspeed_2600_ram_sizes; 416 461 } 417 462 418 463 static const TypeInfo aspeed_2600_sdmc_info = {
+1
include/hw/misc/aspeed_sdmc.h
··· 40 40 SysBusDeviceClass parent_class; 41 41 42 42 uint64_t max_ram_size; 43 + const uint64_t *valid_ram_sizes; 43 44 uint32_t (*compute_conf)(AspeedSDMCState *s, uint32_t data); 44 45 void (*write)(AspeedSDMCState *s, uint32_t reg, uint32_t data); 45 46 } AspeedSDMCClass;