[{"id":1763065,"web_url":"http://patchwork.ozlabs.org/comment/1763065/","msgid":"<20170905102928.6b23a28a.cohuck@redhat.com>","list_archive_url":null,"date":"2017-09-05T08:29:28","subject":"Re: [Qemu-devel] [PATCH v2 1/3] s390x/pci: remove idx from msix msg\n\tdata","submitter":{"id":71914,"url":"http://patchwork.ozlabs.org/api/people/71914/","name":"Cornelia Huck","email":"cohuck@redhat.com"},"content":"On Fri, 1 Sep 2017 06:22:56 +0200\nYi Min Zhao wrote:\n\n> PCIDevice pointer has been a parameter of kvm_arch_fixup_msi_route().\n> So we don't need to store zpci idx in msix message data to find out the\n> specific zpci device. Instead, we could use pci device id to find its\n> corresponding zpci device.\n> \n> Signed-off-by: Yi Min Zhao \n> ---\n> hw/s390x/s390-pci-bus.c | 16 +++++-----------\n> hw/s390x/s390-pci-bus.h | 2 ++\n> hw/s390x/s390-pci-inst.c | 24 ------------------------\n> hw/s390x/s390-pci-stub.c | 6 ++++++\n> target/s390x/kvm.c | 7 +++++--\n> 5 files changed, 18 insertions(+), 37 deletions(-)\n> \n> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c\n> index 0a31a4ae88..bd8a3e1e1c 100644\n> --- a/hw/s390x/s390-pci-bus.c\n> +++ b/hw/s390x/s390-pci-bus.c\n> @@ -199,8 +199,8 @@ static S390PCIBusDevice *s390_pci_find_dev_by_uid(S390pciState *s, uint16_t uid)\n> return NULL;\n> }\n> \n> -static S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,\n> - const char *target)\n> +S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,\n> + const char *target)\n> {\n> S390PCIBusDevice *pbdev;\n> \n> @@ -465,19 +465,13 @@ static void s390_msi_ctrl_write(void *opaque, hwaddr addr, uint64_t data,\n> unsigned int size)\n> {\n> S390PCIBusDevice *pbdev = opaque;\n> - uint32_t idx = data >> ZPCI_MSI_VEC_BITS;\n> uint32_t vec = data & ZPCI_MSI_VEC_MASK;\n> uint64_t ind_bit;\n> uint32_t sum_bit;\n> - uint32_t e = 0;\n> \n> - DPRINTF(\"write_msix data 0x%\" PRIx64 \" idx %d vec 0x%x\\n\", data, idx, vec);\n> -\n> - if (!pbdev) {\n> - e |= (vec << ERR_EVENT_MVN_OFFSET);\n> - s390_pci_generate_error_event(ERR_EVENT_NOMSI, idx, 0, addr, e);\n> - return;\n> - }\n> + assert(pbdev);\n\nI'm wondering whether you could/should generate an error event here.\nThe one above probably won't work (as it seems to take idx as a\nparameter), but is this really 'this must not happen, we messed up in\nour code'? (Probably yes, but I want to be sure.)\n\n> + DPRINTF(\"write_msix data 0x%\" PRIx64 \" idx %d vec 0x%x\\n\", data,\n> + pbdev->idx, vec);\n> \n> if (pbdev->state != ZPCI_FS_ENABLED) {\n> return;\n\n> diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c\n> index 7a642d376c..e501e1b9ea 100644\n> --- a/hw/s390x/s390-pci-stub.c\n> +++ b/hw/s390x/s390-pci-stub.c\n> @@ -74,3 +74,9 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(S390pciState *s, uint32_t idx)\n> {\n> return NULL;\n> }\n\nPlease remove s390_pci_find_dev_by_idx() from the stubs file, as it is\nnot used outside of the conditionally-built pci code anymore.\n\n> +\n> +S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,\n> + const char *target)\n> +{\n> + return NULL;\n> +}\n> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c\n> index 1338c29528..3d490c5e4b 100644\n> --- a/target/s390x/kvm.c\n> +++ b/target/s390x/kvm.c\n> @@ -2533,10 +2533,13 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,\n> uint64_t address, uint32_t data, PCIDevice *dev)\n> {\n> S390PCIBusDevice *pbdev;\n> - uint32_t idx = data >> ZPCI_MSI_VEC_BITS;\n> uint32_t vec = data & ZPCI_MSI_VEC_MASK;\n> \n> - pbdev = s390_pci_find_dev_by_idx(s390_get_phb(), idx);\n> + if (!dev) {\n> + return -ENODEV;\n\nCan this actually happen?\n\n> + }\n> +\n> + pbdev = s390_pci_find_dev_by_target(s390_get_phb(), DEVICE(dev)->id);\n> if (!pbdev) {\n> DPRINTF(\"add_msi_route no dev\\n\");\n> return -ENODEV;","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=cohuck@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xmfzZ1PqQz9s9Y\n\tfor ;\n\tTue, 5 Sep 2017 18:30:18 +1000 (AEST)","from localhost ([::1]:57419 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dp9FQ-0000TW-AB\n\tfor incoming@patchwork.ozlabs.org; Tue, 05 Sep 2017 04:30:16 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:43751)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dp9Eo-0000PU-4A\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 04:29:43 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dp9Ej-0006zm-Bk\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 04:29:38 -0400","from mx1.redhat.com ([209.132.183.28]:47278)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dp9Ej-0006zB-2q\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 04:29:33 -0400","from smtp.corp.redhat.com\n\t(int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id D6B8E1F570;\n\tTue, 5 Sep 2017 08:29:31 +0000 (UTC)","from gondolin (dhcp-192-215.str.redhat.com [10.33.192.215])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 85D237D3E2;\n\tTue, 5 Sep 2017 08:29:30 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com D6B8E1F570","Date":"Tue, 5 Sep 2017 10:29:28 +0200","From":"Cornelia Huck ","To":"Yi Min Zhao ","Message-ID":"<20170905102928.6b23a28a.cohuck@redhat.com>","In-Reply-To":"<1504239778-29893-2-git-send-email-zyimin@linux.vnet.ibm.com>","References":"<1504239778-29893-1-git-send-email-zyimin@linux.vnet.ibm.com>\n\t<1504239778-29893-2-git-send-email-zyimin@linux.vnet.ibm.com>","Organization":"Red Hat GmbH","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.14","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.30]);\n\tTue, 05 Sep 2017 08:29:32 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v2 1/3] s390x/pci: remove idx from msix msg\n\tdata","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"pasic@linux.vnet.ibm.com, pmorel@linux.vnet.ibm.com,\n\trichard.henderson@linaro.org, qemu-devel@nongnu.org,\n\tagraf@suse.de, borntraeger@de.ibm.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1763070,"web_url":"http://patchwork.ozlabs.org/comment/1763070/","msgid":"<0923f97e-4f69-0bf3-b9ab-8e5f69a56bbb@linux.vnet.ibm.com>","list_archive_url":null,"date":"2017-09-05T08:44:37","subject":"Re: [Qemu-devel] [PATCH v2 1/3] s390x/pci: remove idx from msix msg\n\tdata","submitter":{"id":66807,"url":"http://patchwork.ozlabs.org/api/people/66807/","name":"Yi Min Zhao","email":"zyimin@linux.vnet.ibm.com"},"content":"在 2017/9/5 下午4:29, Cornelia Huck 写道:\n> On Fri, 1 Sep 2017 06:22:56 +0200\n> Yi Min Zhao wrote:\n>\n>> PCIDevice pointer has been a parameter of kvm_arch_fixup_msi_route().\n>> So we don't need to store zpci idx in msix message data to find out the\n>> specific zpci device. Instead, we could use pci device id to find its\n>> corresponding zpci device.\n>>\n>> Signed-off-by: Yi Min Zhao \n>> ---\n>> hw/s390x/s390-pci-bus.c | 16 +++++-----------\n>> hw/s390x/s390-pci-bus.h | 2 ++\n>> hw/s390x/s390-pci-inst.c | 24 ------------------------\n>> hw/s390x/s390-pci-stub.c | 6 ++++++\n>> target/s390x/kvm.c | 7 +++++--\n>> 5 files changed, 18 insertions(+), 37 deletions(-)\n>>\n>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c\n>> index 0a31a4ae88..bd8a3e1e1c 100644\n>> --- a/hw/s390x/s390-pci-bus.c\n>> +++ b/hw/s390x/s390-pci-bus.c\n>> @@ -199,8 +199,8 @@ static S390PCIBusDevice *s390_pci_find_dev_by_uid(S390pciState *s, uint16_t uid)\n>> return NULL;\n>> }\n>> \n>> -static S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,\n>> - const char *target)\n>> +S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,\n>> + const char *target)\n>> {\n>> S390PCIBusDevice *pbdev;\n>> \n>> @@ -465,19 +465,13 @@ static void s390_msi_ctrl_write(void *opaque, hwaddr addr, uint64_t data,\n>> unsigned int size)\n>> {\n>> S390PCIBusDevice *pbdev = opaque;\n>> - uint32_t idx = data >> ZPCI_MSI_VEC_BITS;\n>> uint32_t vec = data & ZPCI_MSI_VEC_MASK;\n>> uint64_t ind_bit;\n>> uint32_t sum_bit;\n>> - uint32_t e = 0;\n>> \n>> - DPRINTF(\"write_msix data 0x%\" PRIx64 \" idx %d vec 0x%x\\n\", data, idx, vec);\n>> -\n>> - if (!pbdev) {\n>> - e |= (vec << ERR_EVENT_MVN_OFFSET);\n>> - s390_pci_generate_error_event(ERR_EVENT_NOMSI, idx, 0, addr, e);\n>> - return;\n>> - }\n>> + assert(pbdev);\n> I'm wondering whether you could/should generate an error event here.\n> The one above probably won't work (as it seems to take idx as a\n> parameter), but is this really 'this must not happen, we messed up in\n> our code'? (Probably yes, but I want to be sure.)\nI think this must not happen. One a pci device is plugged into zPCI bus.\nWe would assign a new memory region with zpci device as opaque\nfor its msix. So if s390_msi_ctrl_write() is called, there must be a write\noperation to a pci device's msix ctrl memory region which must has zpci\ndevice as a opaque. The construct is one-msi-mr-per-pci-device.\n>\n>> + DPRINTF(\"write_msix data 0x%\" PRIx64 \" idx %d vec 0x%x\\n\", data,\n>> + pbdev->idx, vec);\n>> \n>> if (pbdev->state != ZPCI_FS_ENABLED) {\n>> return;\n>> diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c\n>> index 7a642d376c..e501e1b9ea 100644\n>> --- a/hw/s390x/s390-pci-stub.c\n>> +++ b/hw/s390x/s390-pci-stub.c\n>> @@ -74,3 +74,9 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(S390pciState *s, uint32_t idx)\n>> {\n>> return NULL;\n>> }\n> Please remove s390_pci_find_dev_by_idx() from the stubs file, as it is\n> not used outside of the conditionally-built pci code anymore.\nI'm confused. s390_pci_find_dev_by_idx() can be called in \nkvm_arch_fixup_msi_route().\nAnd kvm_arch_fixup_msi_route() can be called by kvm_irqchip_add_msi_route().\nAs the code, I think s390_pci_find_dev_by_idx() might be called. Could \nyou please\nexplain more?\n>\n>> +\n>> +S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,\n>> + const char *target)\n>> +{\n>> + return NULL;\n>> +}\n>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c\n>> index 1338c29528..3d490c5e4b 100644\n>> --- a/target/s390x/kvm.c\n>> +++ b/target/s390x/kvm.c\n>> @@ -2533,10 +2533,13 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,\n>> uint64_t address, uint32_t data, PCIDevice *dev)\n>> {\n>> S390PCIBusDevice *pbdev;\n>> - uint32_t idx = data >> ZPCI_MSI_VEC_BITS;\n>> uint32_t vec = data & ZPCI_MSI_VEC_MASK;\n>> \n>> - pbdev = s390_pci_find_dev_by_idx(s390_get_phb(), idx);\n>> + if (!dev) {\n>> + return -ENODEV;\n> Can this actually happen?\nI think this cannot happen. But I'm afraid that I miss something.\nSo I added this to avoid NULL pointer. But from the code and\nmy test, there has not been NULL pointer happened.\n>\n>> + }\n>> +\n>> + pbdev = s390_pci_find_dev_by_target(s390_get_phb(), DEVICE(dev)->id);\n>> if (!pbdev) {\n>> DPRINTF(\"add_msi_route no dev\\n\");\n>> return -ENODEV;\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xmgKG2jKPz9sP3\n\tfor ;\n\tTue, 5 Sep 2017 18:45:35 +1000 (AEST)","from localhost ([::1]:57450 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dp9U5-0003K1-VC\n\tfor incoming@patchwork.ozlabs.org; Tue, 05 Sep 2017 04:45:26 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:49179)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dp9Tb-00039i-NX\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 04:45:00 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dp9TS-0000ec-Is\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 04:44:51 -0400","from mx0b-001b2d01.pphosted.com ([148.163.158.5]:53458\n\thelo=mx0a-001b2d01.pphosted.com)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dp9TS-0000dy-DT\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 04:44:46 -0400","from pps.filterd (m0098417.ppops.net [127.0.0.1])\n\tby mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv858dTaR040567\n\tfor ; Tue, 5 Sep 2017 04:44:45 -0400","from e15.ny.us.ibm.com (e15.ny.us.ibm.com [129.33.205.205])\n\tby mx0a-001b2d01.pphosted.com with ESMTP id 2csq86nd73-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor ; Tue, 05 Sep 2017 04:44:45 -0400","from localhost\n\tby e15.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor from ;\n\tTue, 5 Sep 2017 04:44:44 -0400","from b01cxnp22033.gho.pok.ibm.com (9.57.198.23)\n\tby e15.ny.us.ibm.com (146.89.104.202) with IBM ESMTP SMTP Gateway:\n\tAuthorized Use Only! Violators will be prosecuted; \n\tTue, 5 Sep 2017 04:44:42 -0400","from b01ledav003.gho.pok.ibm.com (b01ledav003.gho.pok.ibm.com\n\t[9.57.199.108])\n\tby b01cxnp22033.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP\n\tid v858ifr323265350; Tue, 5 Sep 2017 08:44:41 GMT","from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 9D794B204E;\n\tTue, 5 Sep 2017 04:42:04 -0400 (EDT)","from zyimindembp.cn.ibm.com (unknown [9.115.193.63])\n\tby b01ledav003.gho.pok.ibm.com (Postfix) with ESMTP id EFAECB2052;\n\tTue, 5 Sep 2017 04:42:01 -0400 (EDT)"],"To":"Cornelia Huck ","References":"<1504239778-29893-1-git-send-email-zyimin@linux.vnet.ibm.com>\n\t<1504239778-29893-2-git-send-email-zyimin@linux.vnet.ibm.com>\n\t<20170905102928.6b23a28a.cohuck@redhat.com>","From":"Yi Min Zhao ","Date":"Tue, 5 Sep 2017 16:44:37 +0800","User-Agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0)\n\tGecko/20100101 Thunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170905102928.6b23a28a.cohuck@redhat.com>","Content-Type":"text/plain; charset=gbk; format=flowed","X-TM-AS-GCONF":"00","x-cbid":"17090508-0036-0000-0000-00000263F863","X-IBM-SpamModules-Scores":"","X-IBM-SpamModules-Versions":"BY=3.00007669; HX=3.00000241; KW=3.00000007;\n\tPH=3.00000004; SC=3.00000226; SDB=6.00912512; UDB=6.00457898;\n\tIPR=6.00692775; \n\tBA=6.00005572; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009;\n\tZB=6.00000000; \n\tZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00017011;\n\tXFM=3.00000015; UTC=2017-09-05 08:44:43","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17090508-0037-0000-0000-000041A905C6","Message-Id":"<0923f97e-4f69-0bf3-b9ab-8e5f69a56bbb@linux.vnet.ibm.com>","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-09-05_03:, , signatures=0","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\tspamscore=0 suspectscore=0\n\tmalwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam\n\tadjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000\n\tdefinitions=main-1709050133","Content-Transfer-Encoding":"quoted-printable","X-MIME-Autoconverted":"from 8bit to quoted-printable by\n\tmx0a-001b2d01.pphosted.com id v858dTaR040567","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy]","X-Received-From":"148.163.158.5","Subject":"Re: [Qemu-devel] [PATCH v2 1/3] s390x/pci: remove idx from msix msg\n\tdata","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"pasic@linux.vnet.ibm.com, pmorel@linux.vnet.ibm.com,\n\trichard.henderson@linaro.org, qemu-devel@nongnu.org,\n\tagraf@suse.de, borntraeger@de.ibm.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1763075,"web_url":"http://patchwork.ozlabs.org/comment/1763075/","msgid":"<20170905105041.3409c8cd.cohuck@redhat.com>","list_archive_url":null,"date":"2017-09-05T08:50:41","subject":"Re: [Qemu-devel] [PATCH v2 1/3] s390x/pci: remove idx from msix msg\n\tdata","submitter":{"id":71914,"url":"http://patchwork.ozlabs.org/api/people/71914/","name":"Cornelia Huck","email":"cohuck@redhat.com"},"content":"On Tue, 5 Sep 2017 16:44:37 +0800\nYi Min Zhao wrote:\n\n> 在 2017/9/5 下午4:29, Cornelia Huck 写道:\n> > On Fri, 1 Sep 2017 06:22:56 +0200\n> > Yi Min Zhao wrote:\n> > \n> >> PCIDevice pointer has been a parameter of kvm_arch_fixup_msi_route().\n> >> So we don't need to store zpci idx in msix message data to find out the\n> >> specific zpci device. Instead, we could use pci device id to find its\n> >> corresponding zpci device.\n> >>\n> >> Signed-off-by: Yi Min Zhao \n> >> ---\n> >> hw/s390x/s390-pci-bus.c | 16 +++++-----------\n> >> hw/s390x/s390-pci-bus.h | 2 ++\n> >> hw/s390x/s390-pci-inst.c | 24 ------------------------\n> >> hw/s390x/s390-pci-stub.c | 6 ++++++\n> >> target/s390x/kvm.c | 7 +++++--\n> >> 5 files changed, 18 insertions(+), 37 deletions(-)\n> >>\n> >> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c\n> >> index 0a31a4ae88..bd8a3e1e1c 100644\n> >> --- a/hw/s390x/s390-pci-bus.c\n> >> +++ b/hw/s390x/s390-pci-bus.c\n> >> @@ -199,8 +199,8 @@ static S390PCIBusDevice *s390_pci_find_dev_by_uid(S390pciState *s, uint16_t uid)\n> >> return NULL;\n> >> }\n> >> \n> >> -static S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,\n> >> - const char *target)\n> >> +S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,\n> >> + const char *target)\n> >> {\n> >> S390PCIBusDevice *pbdev;\n> >> \n> >> @@ -465,19 +465,13 @@ static void s390_msi_ctrl_write(void *opaque, hwaddr addr, uint64_t data,\n> >> unsigned int size)\n> >> {\n> >> S390PCIBusDevice *pbdev = opaque;\n> >> - uint32_t idx = data >> ZPCI_MSI_VEC_BITS;\n> >> uint32_t vec = data & ZPCI_MSI_VEC_MASK;\n> >> uint64_t ind_bit;\n> >> uint32_t sum_bit;\n> >> - uint32_t e = 0;\n> >> \n> >> - DPRINTF(\"write_msix data 0x%\" PRIx64 \" idx %d vec 0x%x\\n\", data, idx, vec);\n> >> -\n> >> - if (!pbdev) {\n> >> - e |= (vec << ERR_EVENT_MVN_OFFSET);\n> >> - s390_pci_generate_error_event(ERR_EVENT_NOMSI, idx, 0, addr, e);\n> >> - return;\n> >> - }\n> >> + assert(pbdev); \n> > I'm wondering whether you could/should generate an error event here.\n> > The one above probably won't work (as it seems to take idx as a\n> > parameter), but is this really 'this must not happen, we messed up in\n> > our code'? (Probably yes, but I want to be sure.) \n> I think this must not happen. One a pci device is plugged into zPCI bus.\n> We would assign a new memory region with zpci device as opaque\n> for its msix. So if s390_msi_ctrl_write() is called, there must be a write\n> operation to a pci device's msix ctrl memory region which must has zpci\n> device as a opaque. The construct is one-msi-mr-per-pci-device.\n\nThis makes sense.\n\n> > \n> >> + DPRINTF(\"write_msix data 0x%\" PRIx64 \" idx %d vec 0x%x\\n\", data,\n> >> + pbdev->idx, vec);\n> >> \n> >> if (pbdev->state != ZPCI_FS_ENABLED) {\n> >> return;\n> >> diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c\n> >> index 7a642d376c..e501e1b9ea 100644\n> >> --- a/hw/s390x/s390-pci-stub.c\n> >> +++ b/hw/s390x/s390-pci-stub.c\n> >> @@ -74,3 +74,9 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(S390pciState *s, uint32_t idx)\n> >> {\n> >> return NULL;\n> >> } \n> > Please remove s390_pci_find_dev_by_idx() from the stubs file, as it is\n> > not used outside of the conditionally-built pci code anymore. \n> I'm confused. s390_pci_find_dev_by_idx() can be called in \n> kvm_arch_fixup_msi_route().\n> And kvm_arch_fixup_msi_route() can be called by kvm_irqchip_add_msi_route().\n> As the code, I think s390_pci_find_dev_by_idx() might be called. Could \n> you please\n> explain more?\n\nBut this patch replaces this with s390_pci_find_dev_by_target(), no?\n\n> > \n> >> +\n> >> +S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,\n> >> + const char *target)\n> >> +{\n> >> + return NULL;\n> >> +}\n> >> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c\n> >> index 1338c29528..3d490c5e4b 100644\n> >> --- a/target/s390x/kvm.c\n> >> +++ b/target/s390x/kvm.c\n> >> @@ -2533,10 +2533,13 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,\n> >> uint64_t address, uint32_t data, PCIDevice *dev)\n> >> {\n> >> S390PCIBusDevice *pbdev;\n> >> - uint32_t idx = data >> ZPCI_MSI_VEC_BITS;\n> >> uint32_t vec = data & ZPCI_MSI_VEC_MASK;\n> >> \n> >> - pbdev = s390_pci_find_dev_by_idx(s390_get_phb(), idx);\n> >> + if (!dev) {\n> >> + return -ENODEV; \n> > Can this actually happen? \n> I think this cannot happen. But I'm afraid that I miss something.\n> So I added this to avoid NULL pointer. But from the code and\n> my test, there has not been NULL pointer happened.\n\nI'm wondering if that is in the same category as the instance I\ncommented on above. Do you want to log something?\n\n> > \n> >> + }\n> >> +\n> >> + pbdev = s390_pci_find_dev_by_target(s390_get_phb(), DEVICE(dev)->id);\n> >> if (!pbdev) {\n> >> DPRINTF(\"add_msi_route no dev\\n\");\n> >> return -ENODEV; \n> > \n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=cohuck@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xmgS33DDfz9sNr\n\tfor ;\n\tTue, 5 Sep 2017 18:51:30 +1000 (AEST)","from localhost ([::1]:57476 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dp9Zv-00061C-L8\n\tfor incoming@patchwork.ozlabs.org; Tue, 05 Sep 2017 04:51:27 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:54872)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dp9ZL-0005xB-N1\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 04:50:56 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dp9ZG-000546-Kb\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 04:50:51 -0400","from mx1.redhat.com ([209.132.183.28]:53902)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dp9ZG-00053g-BQ\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 04:50:46 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 1410F80E72;\n\tTue, 5 Sep 2017 08:50:45 +0000 (UTC)","from gondolin (dhcp-192-215.str.redhat.com [10.33.192.215])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 9C0C08C3A4;\n\tTue, 5 Sep 2017 08:50:43 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 1410F80E72","Date":"Tue, 5 Sep 2017 10:50:41 +0200","From":"Cornelia Huck ","To":"Yi Min Zhao ","Message-ID":"<20170905105041.3409c8cd.cohuck@redhat.com>","In-Reply-To":"<0923f97e-4f69-0bf3-b9ab-8e5f69a56bbb@linux.vnet.ibm.com>","References":"<1504239778-29893-1-git-send-email-zyimin@linux.vnet.ibm.com>\n\t<1504239778-29893-2-git-send-email-zyimin@linux.vnet.ibm.com>\n\t<20170905102928.6b23a28a.cohuck@redhat.com>\n\t<0923f97e-4f69-0bf3-b9ab-8e5f69a56bbb@linux.vnet.ibm.com>","Organization":"Red Hat GmbH","MIME-Version":"1.0","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"quoted-printable","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.25]);\n\tTue, 05 Sep 2017 08:50:45 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v2 1/3] s390x/pci: remove idx from msix msg\n\tdata","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"pasic@linux.vnet.ibm.com, pmorel@linux.vnet.ibm.com,\n\trichard.henderson@linaro.org, qemu-devel@nongnu.org,\n\tagraf@suse.de, borntraeger@de.ibm.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1763105,"web_url":"http://patchwork.ozlabs.org/comment/1763105/","msgid":"<07ef5826-6b09-5ac8-7d51-d00db7170437@linux.vnet.ibm.com>","list_archive_url":null,"date":"2017-09-05T09:08:14","subject":"Re: [Qemu-devel] [PATCH v2 1/3] s390x/pci: remove idx from msix msg\n\tdata","submitter":{"id":66807,"url":"http://patchwork.ozlabs.org/api/people/66807/","name":"Yi Min Zhao","email":"zyimin@linux.vnet.ibm.com"},"content":"在 2017/9/5 下午4:50, Cornelia Huck 写道:\n> On Tue, 5 Sep 2017 16:44:37 +0800\n> Yi Min Zhao wrote:\n>\n>> 在 2017/9/5 下午4:29, Cornelia Huck 写道:\n>>> On Fri, 1 Sep 2017 06:22:56 +0200\n>>> Yi Min Zhao wrote:\n>>> \n>>>> PCIDevice pointer has been a parameter of kvm_arch_fixup_msi_route().\n>>>> So we don't need to store zpci idx in msix message data to find out the\n>>>> specific zpci device. Instead, we could use pci device id to find its\n>>>> corresponding zpci device.\n>>>>\n>>>> Signed-off-by: Yi Min Zhao \n>>>> ---\n>>>> hw/s390x/s390-pci-bus.c | 16 +++++-----------\n>>>> hw/s390x/s390-pci-bus.h | 2 ++\n>>>> hw/s390x/s390-pci-inst.c | 24 ------------------------\n>>>> hw/s390x/s390-pci-stub.c | 6 ++++++\n>>>> target/s390x/kvm.c | 7 +++++--\n>>>> 5 files changed, 18 insertions(+), 37 deletions(-)\n>>>>\n>>>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c\n>>>> index 0a31a4ae88..bd8a3e1e1c 100644\n>>>> --- a/hw/s390x/s390-pci-bus.c\n>>>> +++ b/hw/s390x/s390-pci-bus.c\n>>>> @@ -199,8 +199,8 @@ static S390PCIBusDevice *s390_pci_find_dev_by_uid(S390pciState *s, uint16_t uid)\n>>>> return NULL;\n>>>> }\n>>>> \n>>>> -static S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,\n>>>> - const char *target)\n>>>> +S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,\n>>>> + const char *target)\n>>>> {\n>>>> S390PCIBusDevice *pbdev;\n>>>> \n>>>> @@ -465,19 +465,13 @@ static void s390_msi_ctrl_write(void *opaque, hwaddr addr, uint64_t data,\n>>>> unsigned int size)\n>>>> {\n>>>> S390PCIBusDevice *pbdev = opaque;\n>>>> - uint32_t idx = data >> ZPCI_MSI_VEC_BITS;\n>>>> uint32_t vec = data & ZPCI_MSI_VEC_MASK;\n>>>> uint64_t ind_bit;\n>>>> uint32_t sum_bit;\n>>>> - uint32_t e = 0;\n>>>> \n>>>> - DPRINTF(\"write_msix data 0x%\" PRIx64 \" idx %d vec 0x%x\\n\", data, idx, vec);\n>>>> -\n>>>> - if (!pbdev) {\n>>>> - e |= (vec << ERR_EVENT_MVN_OFFSET);\n>>>> - s390_pci_generate_error_event(ERR_EVENT_NOMSI, idx, 0, addr, e);\n>>>> - return;\n>>>> - }\n>>>> + assert(pbdev);\n>>> I'm wondering whether you could/should generate an error event here.\n>>> The one above probably won't work (as it seems to take idx as a\n>>> parameter), but is this really 'this must not happen, we messed up in\n>>> our code'? (Probably yes, but I want to be sure.)\n>> I think this must not happen. One a pci device is plugged into zPCI bus.\n>> We would assign a new memory region with zpci device as opaque\n>> for its msix. So if s390_msi_ctrl_write() is called, there must be a write\n>> operation to a pci device's msix ctrl memory region which must has zpci\n>> device as a opaque. The construct is one-msi-mr-per-pci-device.\n> This makes sense.\n>\n>>> \n>>>> + DPRINTF(\"write_msix data 0x%\" PRIx64 \" idx %d vec 0x%x\\n\", data,\n>>>> + pbdev->idx, vec);\n>>>> \n>>>> if (pbdev->state != ZPCI_FS_ENABLED) {\n>>>> return;\n>>>> diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c\n>>>> index 7a642d376c..e501e1b9ea 100644\n>>>> --- a/hw/s390x/s390-pci-stub.c\n>>>> +++ b/hw/s390x/s390-pci-stub.c\n>>>> @@ -74,3 +74,9 @@ S390PCIBusDevice *s390_pci_find_dev_by_idx(S390pciState *s, uint32_t idx)\n>>>> {\n>>>> return NULL;\n>>>> }\n>>> Please remove s390_pci_find_dev_by_idx() from the stubs file, as it is\n>>> not used outside of the conditionally-built pci code anymore.\n>> I'm confused. s390_pci_find_dev_by_idx() can be called in\n>> kvm_arch_fixup_msi_route().\n>> And kvm_arch_fixup_msi_route() can be called by kvm_irqchip_add_msi_route().\n>> As the code, I think s390_pci_find_dev_by_idx() might be called. Could\n>> you please\n>> explain more?\n> But this patch replaces this with s390_pci_find_dev_by_target(), no?\nOh! Sorry, I mixed by_target() and by_idx(). Yes, by_idx() should be \nremoved.\n>\n>>> \n>>>> +\n>>>> +S390PCIBusDevice *s390_pci_find_dev_by_target(S390pciState *s,\n>>>> + const char *target)\n>>>> +{\n>>>> + return NULL;\n>>>> +}\n>>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c\n>>>> index 1338c29528..3d490c5e4b 100644\n>>>> --- a/target/s390x/kvm.c\n>>>> +++ b/target/s390x/kvm.c\n>>>> @@ -2533,10 +2533,13 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,\n>>>> uint64_t address, uint32_t data, PCIDevice *dev)\n>>>> {\n>>>> S390PCIBusDevice *pbdev;\n>>>> - uint32_t idx = data >> ZPCI_MSI_VEC_BITS;\n>>>> uint32_t vec = data & ZPCI_MSI_VEC_MASK;\n>>>> \n>>>> - pbdev = s390_pci_find_dev_by_idx(s390_get_phb(), idx);\n>>>> + if (!dev) {\n>>>> + return -ENODEV;\n>>> Can this actually happen?\n>> I think this cannot happen. But I'm afraid that I miss something.\n>> So I added this to avoid NULL pointer. But from the code and\n>> my test, there has not been NULL pointer happened.\n> I'm wondering if that is in the same category as the instance I\n> commented on above. Do you want to log something?\n>\nFor the case above, I ensure that zpci device must exist. But here, I'm \nnot sure.\nBecause it's called from outside. I'm not sure if the caller might call\nkvm_irqchip_add_msi_route() with NULL as pci device argument.\n\nAlthough msix ctrl mr is accessed from outside. But its initialization\nis controled by our code and the pointer to zpci device is saved as\nmr's opaque.\n>>> \n>>>> + }\n>>>> +\n>>>> + pbdev = s390_pci_find_dev_by_target(s390_get_phb(), DEVICE(dev)->id);\n>>>> if (!pbdev) {\n>>>> DPRINTF(\"add_msi_route no dev\\n\");\n>>>> return -ENODEV;\n>>> \n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xmgtJ4qg1z9sPk\n\tfor ;\n\tTue, 5 Sep 2017 19:10:48 +1000 (AEST)","from localhost ([::1]:57581 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dp9sc-0001rO-QU\n\tfor incoming@patchwork.ozlabs.org; Tue, 05 Sep 2017 05:10:46 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:34103)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dp9qP-0000Fq-VI\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:08:35 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dp9qK-0008Py-Ui\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:08:29 -0400","from mx0a-001b2d01.pphosted.com ([148.163.156.1]:48082)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dp9qK-0008PK-Lf\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:08:24 -0400","from pps.filterd (m0098394.ppops.net [127.0.0.1])\n\tby mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv8596ro6007666\n\tfor ; Tue, 5 Sep 2017 05:08:23 -0400","from e18.ny.us.ibm.com (e18.ny.us.ibm.com [129.33.205.208])\n\tby mx0a-001b2d01.pphosted.com with ESMTP id 2csqt5mn7m-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor ; Tue, 05 Sep 2017 05:08:23 -0400","from localhost\n\tby e18.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor from ;\n\tTue, 5 Sep 2017 05:08:22 -0400","from b01cxnp22034.gho.pok.ibm.com (9.57.198.24)\n\tby e18.ny.us.ibm.com (146.89.104.205) with IBM ESMTP SMTP Gateway:\n\tAuthorized Use Only! Violators will be prosecuted; \n\tTue, 5 Sep 2017 05:08:19 -0400","from b01ledav003.gho.pok.ibm.com (b01ledav003.gho.pok.ibm.com\n\t[9.57.199.108])\n\tby b01cxnp22034.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP\n\tid v8598Juh22544568; Tue, 5 Sep 2017 09:08:19 GMT","from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 57B77B2046;\n\tTue, 5 Sep 2017 05:05:42 -0400 (EDT)","from zyimindembp.cn.ibm.com (unknown [9.115.193.63])\n\tby b01ledav003.gho.pok.ibm.com (Postfix) with ESMTP id 78EECB204E;\n\tTue, 5 Sep 2017 05:05:39 -0400 (EDT)"],"To":"Cornelia Huck ","References":"<1504239778-29893-1-git-send-email-zyimin@linux.vnet.ibm.com>\n\t<1504239778-29893-2-git-send-email-zyimin@linux.vnet.ibm.com>\n\t<20170905102928.6b23a28a.cohuck@redhat.com>\n\t<0923f97e-4f69-0bf3-b9ab-8e5f69a56bbb@linux.vnet.ibm.com>\n\t<20170905105041.3409c8cd.cohuck@redhat.com>","From":"Yi Min Zhao ","Date":"Tue, 5 Sep 2017 17:08:14 +0800","User-Agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0)\n\tGecko/20100101 Thunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170905105041.3409c8cd.cohuck@redhat.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","X-TM-AS-GCONF":"00","x-cbid":"17090509-0044-0000-0000-00000388114F","X-IBM-SpamModules-Scores":"","X-IBM-SpamModules-Versions":"BY=3.00007670; HX=3.00000241; KW=3.00000007;\n\tPH=3.00000004; SC=3.00000226; SDB=6.00912520; UDB=6.00457902;\n\tIPR=6.00692782; \n\tBA=6.00005572; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009;\n\tZB=6.00000000; \n\tZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00017011;\n\tXFM=3.00000015; UTC=2017-09-05 09:08:21","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17090509-0045-0000-0000-000007B62BF4","Message-Id":"<07ef5826-6b09-5ac8-7d51-d00db7170437@linux.vnet.ibm.com>","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-09-05_04:, , signatures=0","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\tspamscore=0 suspectscore=0\n\tmalwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam\n\tadjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000\n\tdefinitions=main-1709050139","Content-Transfer-Encoding":"quoted-printable","X-MIME-Autoconverted":"from 8bit to quoted-printable by\n\tmx0a-001b2d01.pphosted.com id v8596ro6007666","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy]","X-Received-From":"148.163.156.1","Subject":"Re: [Qemu-devel] [PATCH v2 1/3] s390x/pci: remove idx from msix msg\n\tdata","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"pasic@linux.vnet.ibm.com, pmorel@linux.vnet.ibm.com,\n\trichard.henderson@linaro.org, qemu-devel@nongnu.org,\n\tagraf@suse.de, borntraeger@de.ibm.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1763112,"web_url":"http://patchwork.ozlabs.org/comment/1763112/","msgid":"<20170905111540.027058d1.cohuck@redhat.com>","list_archive_url":null,"date":"2017-09-05T09:15:40","subject":"Re: [Qemu-devel] [PATCH v2 1/3] s390x/pci: remove idx from msix msg\n\tdata","submitter":{"id":71914,"url":"http://patchwork.ozlabs.org/api/people/71914/","name":"Cornelia Huck","email":"cohuck@redhat.com"},"content":"On Tue, 5 Sep 2017 17:08:14 +0800\nYi Min Zhao wrote:\n\n> 在 2017/9/5 下午4:50, Cornelia Huck 写道:\n> > On Tue, 5 Sep 2017 16:44:37 +0800\n> > Yi Min Zhao wrote:\n> > \n> >> 在 2017/9/5 下午4:29, Cornelia Huck 写道: \n> >>> On Fri, 1 Sep 2017 06:22:56 +0200\n> >>> Yi Min Zhao wrote:\n> >>> \n> >>>> PCIDevice pointer has been a parameter of kvm_arch_fixup_msi_route().\n> >>>> So we don't need to store zpci idx in msix message data to find out the\n> >>>> specific zpci device. Instead, we could use pci device id to find its\n> >>>> corresponding zpci device.\n> >>>>\n> >>>> Signed-off-by: Yi Min Zhao \n> >>>> ---\n> >>>> hw/s390x/s390-pci-bus.c | 16 +++++-----------\n> >>>> hw/s390x/s390-pci-bus.h | 2 ++\n> >>>> hw/s390x/s390-pci-inst.c | 24 ------------------------\n> >>>> hw/s390x/s390-pci-stub.c | 6 ++++++\n> >>>> target/s390x/kvm.c | 7 +++++--\n> >>>> 5 files changed, 18 insertions(+), 37 deletions(-)\n> >>>>\n\n> >>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c\n> >>>> index 1338c29528..3d490c5e4b 100644\n> >>>> --- a/target/s390x/kvm.c\n> >>>> +++ b/target/s390x/kvm.c\n> >>>> @@ -2533,10 +2533,13 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,\n> >>>> uint64_t address, uint32_t data, PCIDevice *dev)\n> >>>> {\n> >>>> S390PCIBusDevice *pbdev;\n> >>>> - uint32_t idx = data >> ZPCI_MSI_VEC_BITS;\n> >>>> uint32_t vec = data & ZPCI_MSI_VEC_MASK;\n> >>>> \n> >>>> - pbdev = s390_pci_find_dev_by_idx(s390_get_phb(), idx);\n> >>>> + if (!dev) {\n> >>>> + return -ENODEV; \n> >>> Can this actually happen? \n> >> I think this cannot happen. But I'm afraid that I miss something.\n> >> So I added this to avoid NULL pointer. But from the code and\n> >> my test, there has not been NULL pointer happened. \n> > I'm wondering if that is in the same category as the instance I\n> > commented on above. Do you want to log something?\n> > \n> For the case above, I ensure that zpci device must exist. But here, I'm \n> not sure.\n> Because it's called from outside. I'm not sure if the caller might call\n> kvm_irqchip_add_msi_route() with NULL as pci device argument.\n> \n> Although msix ctrl mr is accessed from outside. But its initialization\n> is controled by our code and the pointer to zpci device is saved as\n> mr's opaque.\n\nOK. Maybe add a DPRINTF as for the condition below?\n\n> >>> \n> >>>> + }\n> >>>> +\n> >>>> + pbdev = s390_pci_find_dev_by_target(s390_get_phb(), DEVICE(dev)->id);\n> >>>> if (!pbdev) {\n> >>>> DPRINTF(\"add_msi_route no dev\\n\");\n> >>>> return -ENODEV; \n> >>> \n> > \n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=cohuck@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xmh0v49DDz9s7m\n\tfor ;\n\tTue, 5 Sep 2017 19:16:31 +1000 (AEST)","from localhost ([::1]:57629 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dp9y9-0006j3-Mb\n\tfor incoming@patchwork.ozlabs.org; Tue, 05 Sep 2017 05:16:29 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:38176)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dp9xa-0006ej-7J\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:15:59 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dp9xU-0005KE-DI\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:15:53 -0400","from mx1.redhat.com ([209.132.183.28]:51242)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dp9xT-0005GC-MG\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:15:48 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 4E6C881DF6;\n\tTue, 5 Sep 2017 09:15:44 +0000 (UTC)","from gondolin (dhcp-192-215.str.redhat.com [10.33.192.215])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id F287017265;\n\tTue, 5 Sep 2017 09:15:42 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 4E6C881DF6","Date":"Tue, 5 Sep 2017 11:15:40 +0200","From":"Cornelia Huck ","To":"Yi Min Zhao ","Message-ID":"<20170905111540.027058d1.cohuck@redhat.com>","In-Reply-To":"<07ef5826-6b09-5ac8-7d51-d00db7170437@linux.vnet.ibm.com>","References":"<1504239778-29893-1-git-send-email-zyimin@linux.vnet.ibm.com>\n\t<1504239778-29893-2-git-send-email-zyimin@linux.vnet.ibm.com>\n\t<20170905102928.6b23a28a.cohuck@redhat.com>\n\t<0923f97e-4f69-0bf3-b9ab-8e5f69a56bbb@linux.vnet.ibm.com>\n\t<20170905105041.3409c8cd.cohuck@redhat.com>\n\t<07ef5826-6b09-5ac8-7d51-d00db7170437@linux.vnet.ibm.com>","Organization":"Red Hat GmbH","MIME-Version":"1.0","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"quoted-printable","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.25]);\n\tTue, 05 Sep 2017 09:15:44 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v2 1/3] s390x/pci: remove idx from msix msg\n\tdata","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"pasic@linux.vnet.ibm.com, pmorel@linux.vnet.ibm.com,\n\trichard.henderson@linaro.org, qemu-devel@nongnu.org,\n\tagraf@suse.de, borntraeger@de.ibm.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1763117,"web_url":"http://patchwork.ozlabs.org/comment/1763117/","msgid":"<65b4c93c-fc2e-7ba3-b6e2-aa6c2312df3b@linux.vnet.ibm.com>","list_archive_url":null,"date":"2017-09-05T09:21:52","subject":"Re: [Qemu-devel] [PATCH v2 1/3] s390x/pci: remove idx from msix msg\n\tdata","submitter":{"id":66807,"url":"http://patchwork.ozlabs.org/api/people/66807/","name":"Yi Min Zhao","email":"zyimin@linux.vnet.ibm.com"},"content":"在 2017/9/5 下午5:15, Cornelia Huck 写道:\n> On Tue, 5 Sep 2017 17:08:14 +0800\n> Yi Min Zhao wrote:\n>\n>> 在 2017/9/5 下午4:50, Cornelia Huck 写道:\n>>> On Tue, 5 Sep 2017 16:44:37 +0800\n>>> Yi Min Zhao wrote:\n>>> \n>>>> 在 2017/9/5 下午4:29, Cornelia Huck 写道:\n>>>>> On Fri, 1 Sep 2017 06:22:56 +0200\n>>>>> Yi Min Zhao wrote:\n>>>>> \n>>>>>> PCIDevice pointer has been a parameter of kvm_arch_fixup_msi_route().\n>>>>>> So we don't need to store zpci idx in msix message data to find out the\n>>>>>> specific zpci device. Instead, we could use pci device id to find its\n>>>>>> corresponding zpci device.\n>>>>>>\n>>>>>> Signed-off-by: Yi Min Zhao \n>>>>>> ---\n>>>>>> hw/s390x/s390-pci-bus.c | 16 +++++-----------\n>>>>>> hw/s390x/s390-pci-bus.h | 2 ++\n>>>>>> hw/s390x/s390-pci-inst.c | 24 ------------------------\n>>>>>> hw/s390x/s390-pci-stub.c | 6 ++++++\n>>>>>> target/s390x/kvm.c | 7 +++++--\n>>>>>> 5 files changed, 18 insertions(+), 37 deletions(-)\n>>>>>>\n>>>>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c\n>>>>>> index 1338c29528..3d490c5e4b 100644\n>>>>>> --- a/target/s390x/kvm.c\n>>>>>> +++ b/target/s390x/kvm.c\n>>>>>> @@ -2533,10 +2533,13 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,\n>>>>>> uint64_t address, uint32_t data, PCIDevice *dev)\n>>>>>> {\n>>>>>> S390PCIBusDevice *pbdev;\n>>>>>> - uint32_t idx = data >> ZPCI_MSI_VEC_BITS;\n>>>>>> uint32_t vec = data & ZPCI_MSI_VEC_MASK;\n>>>>>> \n>>>>>> - pbdev = s390_pci_find_dev_by_idx(s390_get_phb(), idx);\n>>>>>> + if (!dev) {\n>>>>>> + return -ENODEV;\n>>>>> Can this actually happen?\n>>>> I think this cannot happen. But I'm afraid that I miss something.\n>>>> So I added this to avoid NULL pointer. But from the code and\n>>>> my test, there has not been NULL pointer happened.\n>>> I'm wondering if that is in the same category as the instance I\n>>> commented on above. Do you want to log something?\n>>> \n>> For the case above, I ensure that zpci device must exist. But here, I'm\n>> not sure.\n>> Because it's called from outside. I'm not sure if the caller might call\n>> kvm_irqchip_add_msi_route() with NULL as pci device argument.\n>>\n>> Although msix ctrl mr is accessed from outside. But its initialization\n>> is controled by our code and the pointer to zpci device is saved as\n>> mr's opaque.\n> OK. Maybe add a DPRINTF as for the condition below?\nOK. How about DPRINTF(\"add_msi_route no pci device\\n\")?\nAnd change the DPRINTF for the below condition to\nDPRINTF(\"add_msi_route no zpci device\\n\").\n>\n>>>>> \n>>>>>> + }\n>>>>>> +\n>>>>>> + pbdev = s390_pci_find_dev_by_target(s390_get_phb(), DEVICE(dev)->id);\n>>>>>> if (!pbdev) {\n>>>>>> DPRINTF(\"add_msi_route no dev\\n\");\n>>>>>> return -ENODEV;\n>>>>> \n>>> \n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xmh8D6dFTz9sNr\n\tfor ;\n\tTue, 5 Sep 2017 19:22:50 +1000 (AEST)","from localhost ([::1]:57658 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dpA4F-0000pa-SL\n\tfor incoming@patchwork.ozlabs.org; Tue, 05 Sep 2017 05:22:47 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:40904)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dpA3d-0000pG-45\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:22:14 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dpA3V-0002Os-FP\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:22:09 -0400","from mx0a-001b2d01.pphosted.com ([148.163.156.1]:41790)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dpA3V-0002O4-5Y\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:22:01 -0400","from pps.filterd (m0098409.ppops.net [127.0.0.1])\n\tby mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv859JjgQ093648\n\tfor ; Tue, 5 Sep 2017 05:21:59 -0400","from e23smtp08.au.ibm.com (e23smtp08.au.ibm.com [202.81.31.141])\n\tby mx0a-001b2d01.pphosted.com with ESMTP id 2csqchpyhm-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor ; Tue, 05 Sep 2017 05:21:59 -0400","from localhost\n\tby e23smtp08.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor from ;\n\tTue, 5 Sep 2017 19:21:56 +1000","from d23relay08.au.ibm.com (202.81.31.227)\n\tby e23smtp08.au.ibm.com (202.81.31.205) with IBM ESMTP SMTP Gateway:\n\tAuthorized Use Only! Violators will be prosecuted; \n\tTue, 5 Sep 2017 19:21:55 +1000","from d23av06.au.ibm.com (d23av06.au.ibm.com [9.190.235.151])\n\tby d23relay08.au.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id\n\tv859LtpI42991658\n\tfor ; Tue, 5 Sep 2017 19:21:55 +1000","from d23av06.au.ibm.com (localhost [127.0.0.1])\n\tby d23av06.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id\n\tv859Lsu9024673\n\tfor ; Tue, 5 Sep 2017 19:21:55 +1000","from zyimindembp.cn.ibm.com (zyimindembp.cn.ibm.com [9.115.193.63])\n\tby d23av06.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id\n\tv859Lqb8024624; Tue, 5 Sep 2017 19:21:53 +1000"],"To":"Cornelia Huck ","References":"<1504239778-29893-1-git-send-email-zyimin@linux.vnet.ibm.com>\n\t<1504239778-29893-2-git-send-email-zyimin@linux.vnet.ibm.com>\n\t<20170905102928.6b23a28a.cohuck@redhat.com>\n\t<0923f97e-4f69-0bf3-b9ab-8e5f69a56bbb@linux.vnet.ibm.com>\n\t<20170905105041.3409c8cd.cohuck@redhat.com>\n\t<07ef5826-6b09-5ac8-7d51-d00db7170437@linux.vnet.ibm.com>\n\t<20170905111540.027058d1.cohuck@redhat.com>","From":"Yi Min Zhao ","Date":"Tue, 5 Sep 2017 17:21:52 +0800","User-Agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0)\n\tGecko/20100101 Thunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170905111540.027058d1.cohuck@redhat.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","X-TM-AS-MML":"disable","x-cbid":"17090509-0048-0000-0000-0000025AD653","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17090509-0049-0000-0000-0000480F3257","Message-Id":"<65b4c93c-fc2e-7ba3-b6e2-aa6c2312df3b@linux.vnet.ibm.com>","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-09-05_04:, , signatures=0","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\tspamscore=0 suspectscore=0\n\tmalwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam\n\tadjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000\n\tdefinitions=main-1709050144","Content-Transfer-Encoding":"quoted-printable","X-MIME-Autoconverted":"from 8bit to quoted-printable by\n\tmx0a-001b2d01.pphosted.com id v859JjgQ093648","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy]","X-Received-From":"148.163.156.1","Subject":"Re: [Qemu-devel] [PATCH v2 1/3] s390x/pci: remove idx from msix msg\n\tdata","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"pasic@linux.vnet.ibm.com, pmorel@linux.vnet.ibm.com,\n\trichard.henderson@linaro.org, qemu-devel@nongnu.org,\n\tagraf@suse.de, borntraeger@de.ibm.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1763125,"web_url":"http://patchwork.ozlabs.org/comment/1763125/","msgid":"<20170905112533.31e576b5.cohuck@redhat.com>","list_archive_url":null,"date":"2017-09-05T09:25:33","subject":"Re: [Qemu-devel] [PATCH v2 1/3] s390x/pci: remove idx from msix msg\n\tdata","submitter":{"id":71914,"url":"http://patchwork.ozlabs.org/api/people/71914/","name":"Cornelia Huck","email":"cohuck@redhat.com"},"content":"On Tue, 5 Sep 2017 17:21:52 +0800\nYi Min Zhao wrote:\n\n> 在 2017/9/5 下午5:15, Cornelia Huck 写道:\n> > On Tue, 5 Sep 2017 17:08:14 +0800\n> > Yi Min Zhao wrote:\n> > \n> >> 在 2017/9/5 下午4:50, Cornelia Huck 写道: \n> >>> On Tue, 5 Sep 2017 16:44:37 +0800\n> >>> Yi Min Zhao wrote:\n> >>> \n> >>>> 在 2017/9/5 下午4:29, Cornelia Huck 写道: \n> >>>>> On Fri, 1 Sep 2017 06:22:56 +0200\n> >>>>> Yi Min Zhao wrote:\n> >>>>> \n> >>>>>> PCIDevice pointer has been a parameter of kvm_arch_fixup_msi_route().\n> >>>>>> So we don't need to store zpci idx in msix message data to find out the\n> >>>>>> specific zpci device. Instead, we could use pci device id to find its\n> >>>>>> corresponding zpci device.\n> >>>>>>\n> >>>>>> Signed-off-by: Yi Min Zhao \n> >>>>>> ---\n> >>>>>> hw/s390x/s390-pci-bus.c | 16 +++++-----------\n> >>>>>> hw/s390x/s390-pci-bus.h | 2 ++\n> >>>>>> hw/s390x/s390-pci-inst.c | 24 ------------------------\n> >>>>>> hw/s390x/s390-pci-stub.c | 6 ++++++\n> >>>>>> target/s390x/kvm.c | 7 +++++--\n> >>>>>> 5 files changed, 18 insertions(+), 37 deletions(-)\n> >>>>>>\n> >>>>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c\n> >>>>>> index 1338c29528..3d490c5e4b 100644\n> >>>>>> --- a/target/s390x/kvm.c\n> >>>>>> +++ b/target/s390x/kvm.c\n> >>>>>> @@ -2533,10 +2533,13 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,\n> >>>>>> uint64_t address, uint32_t data, PCIDevice *dev)\n> >>>>>> {\n> >>>>>> S390PCIBusDevice *pbdev;\n> >>>>>> - uint32_t idx = data >> ZPCI_MSI_VEC_BITS;\n> >>>>>> uint32_t vec = data & ZPCI_MSI_VEC_MASK;\n> >>>>>> \n> >>>>>> - pbdev = s390_pci_find_dev_by_idx(s390_get_phb(), idx);\n> >>>>>> + if (!dev) {\n> >>>>>> + return -ENODEV; \n> >>>>> Can this actually happen? \n> >>>> I think this cannot happen. But I'm afraid that I miss something.\n> >>>> So I added this to avoid NULL pointer. But from the code and\n> >>>> my test, there has not been NULL pointer happened. \n> >>> I'm wondering if that is in the same category as the instance I\n> >>> commented on above. Do you want to log something?\n> >>> \n> >> For the case above, I ensure that zpci device must exist. But here, I'm\n> >> not sure.\n> >> Because it's called from outside. I'm not sure if the caller might call\n> >> kvm_irqchip_add_msi_route() with NULL as pci device argument.\n> >>\n> >> Although msix ctrl mr is accessed from outside. But its initialization\n> >> is controled by our code and the pointer to zpci device is saved as\n> >> mr's opaque. \n> > OK. Maybe add a DPRINTF as for the condition below? \n> OK. How about DPRINTF(\"add_msi_route no pci device\\n\")?\n> And change the DPRINTF for the below condition to\n> DPRINTF(\"add_msi_route no zpci device\\n\").\n\nworks for me\n\n> > \n> >>>>> \n> >>>>>> + }\n> >>>>>> +\n> >>>>>> + pbdev = s390_pci_find_dev_by_target(s390_get_phb(), DEVICE(dev)->id);\n> >>>>>> if (!pbdev) {\n> >>>>>> DPRINTF(\"add_msi_route no dev\\n\");\n> >>>>>> return -ENODEV; \n> >>>>> \n> >>> \n> > \n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=cohuck@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xmhHV4jJ4z9s75\n\tfor ;\n\tTue, 5 Sep 2017 19:29:10 +1000 (AEST)","from localhost ([::1]:57688 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dpAAO-0006Ho-Jk\n\tfor incoming@patchwork.ozlabs.org; Tue, 05 Sep 2017 05:29:08 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:43138)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dpA74-0003uh-Ku\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:25:48 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dpA6z-0004nK-Fu\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:25:42 -0400","from mx1.redhat.com ([209.132.183.28]:56024)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dpA6z-0004mT-77\n\tfor qemu-devel@nongnu.org; Tue, 05 Sep 2017 05:25:37 -0400","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 293275F739;\n\tTue, 5 Sep 2017 09:25:36 +0000 (UTC)","from gondolin (dhcp-192-215.str.redhat.com [10.33.192.215])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id C840984D61;\n\tTue, 5 Sep 2017 09:25:34 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 293275F739","Date":"Tue, 5 Sep 2017 11:25:33 +0200","From":"Cornelia Huck ","To":"Yi Min Zhao ","Message-ID":"<20170905112533.31e576b5.cohuck@redhat.com>","In-Reply-To":"<65b4c93c-fc2e-7ba3-b6e2-aa6c2312df3b@linux.vnet.ibm.com>","References":"<1504239778-29893-1-git-send-email-zyimin@linux.vnet.ibm.com>\n\t<1504239778-29893-2-git-send-email-zyimin@linux.vnet.ibm.com>\n\t<20170905102928.6b23a28a.cohuck@redhat.com>\n\t<0923f97e-4f69-0bf3-b9ab-8e5f69a56bbb@linux.vnet.ibm.com>\n\t<20170905105041.3409c8cd.cohuck@redhat.com>\n\t<07ef5826-6b09-5ac8-7d51-d00db7170437@linux.vnet.ibm.com>\n\t<20170905111540.027058d1.cohuck@redhat.com>\n\t<65b4c93c-fc2e-7ba3-b6e2-aa6c2312df3b@linux.vnet.ibm.com>","Organization":"Red Hat GmbH","MIME-Version":"1.0","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"quoted-printable","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.39]);\n\tTue, 05 Sep 2017 09:25:36 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v2 1/3] s390x/pci: remove idx from msix msg\n\tdata","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"pasic@linux.vnet.ibm.com, pmorel@linux.vnet.ibm.com,\n\trichard.henderson@linaro.org, qemu-devel@nongnu.org,\n\tagraf@suse.de, borntraeger@de.ibm.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]