linux-mips
[Top] [All Lists]

Re: [PATCH v3 11/16] KVM: Move vcpu_load to arch-specific kvm_arch_vcpu_

To: Cornelia Huck <cohuck@redhat.com>, Christoffer Dall <cdall@kernel.org>
Subject: Re: [PATCH v3 11/16] KVM: Move vcpu_load to arch-specific kvm_arch_vcpu_ioctl_set_guest_debug
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Tue, 12 Dec 2017 17:36:34 +0100
Cc: linux-mips@linux-mips.org, kvm@vger.kernel.org, Marc Zyngier <marc.zyngier@arm.com>, James Hogan <jhogan@kernel.org>, kvm-ppc@vger.kernel.org, Paul Mackerras <paulus@ozlabs.org>, Christian Borntraeger <borntraeger@de.ibm.com>, linux-arm-kernel@lists.infradead.org, linux-s390@vger.kernel.org, kvmarm@lists.cs.columbia.edu
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=b6PD1hcAA+b82mAK6NrApmGxfXkyMCpPsYOA2ZA4k50=; b=V5AG90GPRlWL03zh61C3p/T9rkRKKIv6XtUeb4ktIViF2Dd9zEf7VMO0HWf4w+wce9 nrcV0fYaLGQLYrsYG+fkl4ZhMNcxL60NTHErmp4NxQwYcBq9U9fKAMsr5LtrAgmWJvft BADIQS7ZjB1ErgquYPjfSZHIT1APK+lJFsDAd3tEBlhAQWPm5WQeu4myZqvG4L5K2tOB BJdsu8DDCt/0MIX9L2mPhbIMJZ7ydWWm46GIxb7FYos5ADAksGjdMdiMzB6vlHKEU9/S le8UTzHMpBRv6PVR3qFqVGeK7fPKup4owoRzRSH9CWmJNm0sysSZGwYAjQLGdyL3f8D/ Zo1A==
In-reply-to: <20171211133943.236f18be.cohuck@redhat.com>
List-archive: <http://www.linux-mips.org/archives/linux-mips/>
List-help: <mailto:ecartis@linux-mips.org?Subject=help>
List-id: linux-mips <linux-mips.eddie.linux-mips.org>
List-owner: <mailto:ralf@linux-mips.org>
List-post: <mailto:linux-mips@linux-mips.org>
List-software: Ecartis version 1.0.0
List-subscribe: <mailto:ecartis@linux-mips.org?subject=subscribe%20linux-mips>
List-unsubscribe: <mailto:ecartis@linux-mips.org?subject=unsubscribe%20linux-mips>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <20171204203538.8370-1-cdall@kernel.org> <20171204203538.8370-12-cdall@kernel.org> <20171211133943.236f18be.cohuck@redhat.com>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0
On 11/12/2017 13:39, Cornelia Huck wrote:
>> +    ret = -EINVAL;
>>      for (n = 0; n < (KVMPPC_BOOKE_IAC_NUM + KVMPPC_BOOKE_DAC_NUM); n++) {
>>              uint64_t addr = dbg->arch.bp[n].addr;
>>              uint32_t type = dbg->arch.bp[n].type;
>> @@ -2067,21 +2071,24 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct 
>> kvm_vcpu *vcpu,
>>              if (type & ~(KVMPPC_DEBUG_WATCH_READ |
>>                           KVMPPC_DEBUG_WATCH_WRITE |
>>                           KVMPPC_DEBUG_BREAKPOINT))
>> -                    return -EINVAL;
>> +                    goto out;
>>  
>>              if (type & KVMPPC_DEBUG_BREAKPOINT) {
>>                      /* Setting H/W breakpoint */
>>                      if (kvmppc_booke_add_breakpoint(dbg_reg, addr, b++))
>> -                            return -EINVAL;
>> +                            goto out;
>>              } else {
>>                      /* Setting H/W watchpoint */
>>                      if (kvmppc_booke_add_watchpoint(dbg_reg, addr,
>>                                                      type, w++))
>> -                            return -EINVAL;
>> +                            goto out;
>>              }
>>      }
>>  
>> -    return 0;
>> +    ret = 0;
> 
> I would probably set the -EINVAL in the individual branches (so it is
> clear that something is wrong, and it is not just a benign exit as in
> the cases above), but your code is correct as well. Let the powerpc
> folks decide.

The idiom that Christoffer used is found elsewhere in KVM, so I'm
accepting his version.  Thanks for the review!

Paolo

<Prev in Thread] Current Thread [Next in Thread>