On 05/08/2021 17.28, David Hildenbrand wrote:
> Let's validate the given address and report a proper error in case it's
> not. All call paths now properly check the validity of the given GFN.
> Remove the TODO.
>
> The errors inside the getter and setter should only trigger if something
> really goes wrong now, for example, with a broken migration stream. Or
> when we forget to update the storage key allocation with memory hotplug.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> hw/s390x/s390-skeys.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
> index 56a47fe180..53e16f1b9c 100644
> --- a/hw/s390x/s390-skeys.c
> +++ b/hw/s390x/s390-skeys.c
> @@ -18,6 +18,7 @@
> #include "qapi/qmp/qdict.h"
> #include "qemu/error-report.h"
> #include "sysemu/memory_mapping.h"
> +#include "exec/address-spaces.h"
> #include "sysemu/kvm.h"
> #include "migration/qemu-file-types.h"
> #include "migration/register.h"
> @@ -86,6 +87,12 @@ void hmp_info_skeys(Monitor *mon, const QDict *qdict)
> return;
> }
>
> + if (!address_space_access_valid(&address_space_memory,
> + addr & TARGET_PAGE_MASK, TARGET_PAGE_SIZE,
> + false, MEMTXATTRS_UNSPECIFIED)) {
> + monitor_printf(mon, "Error: The given address is not valid\n");
I think the code should return here?
> + }
> +
> r = skeyclass->get_skeys(ss, addr / TARGET_PAGE_SIZE, 1, &key);
> if (r < 0) {
> monitor_printf(mon, "Error: %s\n", strerror(-r));
> @@ -197,11 +204,6 @@ static int qemu_s390_skeys_enabled(S390SKeysState *ss)
> return 1;
> }
>
> -/*
> - * TODO: for memory hotplug support qemu_s390_skeys_set and qemu_s390_skeys_get
> - * will have to make sure that the given gfn belongs to a memory region and not
> - * a memory hole.
> - */
> static int qemu_s390_skeys_set(S390SKeysState *ss, uint64_t start_gfn,
> uint64_t count, uint8_t *keys)
> {
>
Thomas