[{"id":1764179,"web_url":"http://patchwork.ozlabs.org/comment/1764179/","msgid":"<20170906144958.GA7570@localhost.localdomain>","list_archive_url":null,"date":"2017-09-06T14:49:58","subject":"Re: [Qemu-devel] [PATCH 2/2] hw/pcie: disable IO port fwd by\n\tdefault for pcie-root-port","submitter":{"id":195,"url":"http://patchwork.ozlabs.org/api/people/195/","name":"Eduardo Habkost","email":"ehabkost@redhat.com"},"content":"On Wed, Sep 06, 2017 at 05:26:58PM +0300, Marcel Apfelbaum wrote:\n> For most cases the devices attached to PCIe Root Ports\n> do not need IO ports range, add an 'enable-io-fwd' property\n> making it false by default, but keeping it true for older machines.\n> \n> Signed-off-by: Marcel Apfelbaum \n[...]\n> @@ -78,6 +111,7 @@ static const VMStateDescription vmstate_rp_dev = {\n> \n> static Property gen_rp_props[] = {\n> DEFINE_PROP_BOOL(\"x-migrate-msix\", GenPCIERootPort, migrate_msix, true),\n> + DEFINE_PROP_BOOL(\"enable-io-fwd\", GenPCIERootPort, enable_io_fwd, false),\n\nThere's no \"x-\" prefix, so I guess we really need to let the user\nor management software to set enable-io-fwd=on explicitly on some\ncases?","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-mx03.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx03.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=ehabkost@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 3xnRd113wwz9t5C\n\tfor ;\n\tThu, 7 Sep 2017 01:01:56 +1000 (AEST)","from localhost ([::1]:36574 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dpbpy-0006w8-6o\n\tfor incoming@patchwork.ozlabs.org; Wed, 06 Sep 2017 11:01:54 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:40216)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dpbek-0005nB-HZ\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 10:50:23 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dpbeg-0000mD-08\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 10:50:18 -0400","from mx1.redhat.com ([209.132.183.28]:33172)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dpbef-0000lp-QB\n\tfor qemu-devel@nongnu.org; Wed, 06 Sep 2017 10:50:13 -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 F330B7E424;\n\tWed, 6 Sep 2017 14:50:11 +0000 (UTC)","from localhost (ovpn-116-45.gru2.redhat.com [10.97.116.45])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 41944835E1;\n\tWed, 6 Sep 2017 14:50:00 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com F330B7E424","Date":"Wed, 6 Sep 2017 11:49:58 -0300","From":"Eduardo Habkost ","To":"Marcel Apfelbaum ","Message-ID":"<20170906144958.GA7570@localhost.localdomain>","References":"<20170906142658.58298-1-marcel@redhat.com>\n\t<20170906142658.58298-3-marcel@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170906142658.58298-3-marcel@redhat.com>","X-Fnord":"you can see the fnord","User-Agent":"Mutt/1.8.3 (2017-05-23)","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.27]);\n\tWed, 06 Sep 2017 14:50:12 +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 2/2] hw/pcie: disable IO port fwd by\n\tdefault for pcie-root-port","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":"pbonzini@redhat.com, rth@twiddle.net, qemu-devel@nongnu.org,\n\tmst@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1765268,"web_url":"http://patchwork.ozlabs.org/comment/1765268/","msgid":"<0c764f20-16ed-5bbc-f792-e410f56f1c93@redhat.com>","list_archive_url":null,"date":"2017-09-08T11:39:04","subject":"Re: [Qemu-devel] [PATCH 2/2] hw/pcie: disable IO port fwd by\n\tdefault for pcie-root-port","submitter":{"id":65362,"url":"http://patchwork.ozlabs.org/api/people/65362/","name":"Marcel Apfelbaum","email":"marcel@redhat.com"},"content":"On 06/09/2017 17:49, Eduardo Habkost wrote:\n> On Wed, Sep 06, 2017 at 05:26:58PM +0300, Marcel Apfelbaum wrote:\n>> For most cases the devices attached to PCIe Root Ports\n>> do not need IO ports range, add an 'enable-io-fwd' property\n>> making it false by default, but keeping it true for older machines.\n>>\n>> Signed-off-by: Marcel Apfelbaum \n> [...]\n>> @@ -78,6 +111,7 @@ static const VMStateDescription vmstate_rp_dev = {\n>> \n>> static Property gen_rp_props[] = {\n>> DEFINE_PROP_BOOL(\"x-migrate-msix\", GenPCIERootPort, migrate_msix, true),\n>> + DEFINE_PROP_BOOL(\"enable-io-fwd\", GenPCIERootPort, enable_io_fwd, false),\n> \n\nHi Eduardo,\n\n> There's no \"x-\" prefix, so I guess we really need to let the user\n> or management software to set enable-io-fwd=on explicitly on some\n> cases?\n> \n\nDefinitely. Specifically 2 cases:\n - attach PCIe-PCI bridge to start a legacy PCI hierarchy.\n - assign a device requiring IO ports range.\n\nThanks,\nMarcel","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-mx02.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=marcel@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 3xpb2r1wWQz9s83\n\tfor ;\n\tFri, 8 Sep 2017 21:39:47 +1000 (AEST)","from localhost ([::1]:44828 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dqHdQ-0006ku-Og\n\tfor incoming@patchwork.ozlabs.org; Fri, 08 Sep 2017 07:39:44 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:57540)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dqHd3-0006kb-1V\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 07:39:21 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dqHcz-0002mM-Qv\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 07:39:21 -0400","from mx1.redhat.com ([209.132.183.28]:41636)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dqHcz-0002l3-L1\n\tfor qemu-devel@nongnu.org; Fri, 08 Sep 2017 07:39:17 -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 AD5AB883BF;\n\tFri, 8 Sep 2017 11:39:15 +0000 (UTC)","from mapfelba-osx.local (unknown [10.35.206.25])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 118E65C8B7;\n\tFri, 8 Sep 2017 11:39:07 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com AD5AB883BF","To":"Eduardo Habkost ","References":"<20170906142658.58298-1-marcel@redhat.com>\n\t<20170906142658.58298-3-marcel@redhat.com>\n\t<20170906144958.GA7570@localhost.localdomain>","From":"Marcel Apfelbaum ","Message-ID":"<0c764f20-16ed-5bbc-f792-e410f56f1c93@redhat.com>","Date":"Fri, 8 Sep 2017 14:39:04 +0300","User-Agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0)\n\tGecko/20100101 Thunderbird/52.1.1","MIME-Version":"1.0","In-Reply-To":"<20170906144958.GA7570@localhost.localdomain>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","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.26]);\n\tFri, 08 Sep 2017 11:39:15 +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 2/2] hw/pcie: disable IO port fwd by\n\tdefault for pcie-root-port","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":"mst@redhat.com, Alexander Bezzubikov ,\n\tqemu-devel@nongnu.org, Laine Stump ,\n\tpbonzini@redhat.com, rth@twiddle.net","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1771428,"web_url":"http://patchwork.ozlabs.org/comment/1771428/","msgid":"<4ca77991-ad6a-a138-622b-b42f410fa1eb@redhat.com>","list_archive_url":null,"date":"2017-09-19T22:15:36","subject":"Re: [Qemu-devel] [PATCH 2/2] hw/pcie: disable IO port fwd by\n\tdefault for pcie-root-port","submitter":{"id":9243,"url":"http://patchwork.ozlabs.org/api/people/9243/","name":"Laszlo Ersek","email":"lersek@redhat.com"},"content":"Hi Marcel,\n\nOn 09/06/17 16:26, Marcel Apfelbaum wrote:\n> For most cases the devices attached to PCIe Root Ports\n> do not need IO ports range, add an 'enable-io-fwd' property\n> making it false by default, but keeping it true for older machines.\n> \n> Signed-off-by: Marcel Apfelbaum \n> ---\n> hw/pci-bridge/gen_pcie_root_port.c | 38 ++++++++++++++++++++++++++++++++++++++\n> include/hw/compat.h | 6 +++++-\n> 2 files changed, 43 insertions(+), 1 deletion(-)\n> \n> diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c\n> index cb694d6da5..cbdeb73e2c 100644\n> --- a/hw/pci-bridge/gen_pcie_root_port.c\n> +++ b/hw/pci-bridge/gen_pcie_root_port.c\n> @@ -20,12 +20,26 @@\n> #define GEN_PCIE_ROOT_PORT_AER_OFFSET 0x100\n> #define GEN_PCIE_ROOT_PORT_MSIX_NR_VECTOR 1\n> \n> +#define GEN_PCIE_ROOT_PORT(obj) \\\n> + OBJECT_CHECK(GenPCIERootPort, (obj), TYPE_GEN_PCIE_ROOT_PORT)\n> +#define GEN_PCIE_ROOT_PORT_CLASS(klass) \\\n> + OBJECT_CLASS_CHECK(GenPCIERootPortClass, (klass), TYPE_GEN_PCIE_ROOT_PORT)\n> +#define GEN_PCIE_ROOT_PORT_GET_CLASS(obj) \\\n> + OBJECT_GET_CLASS(GenPCIERootPortClass, (obj), TYPE_GEN_PCIE_ROOT_PORT)\n> +\n> +typedef struct GenPCIERootPortClass {\n> + PCIERootPortClass parent_class;\n> +\n> + DeviceRealize parent_realize;\n> +} GenPCIERootPortClass;\n> +\n> typedef struct GenPCIERootPort {\n> /*< private >*/\n> PCIESlot parent_obj;\n> /*< public >*/\n> \n> bool migrate_msix;\n> + bool enable_io_fwd;\n> } GenPCIERootPort;\n> \n> static uint8_t gen_rp_aer_vector(const PCIDevice *d)\n> @@ -60,6 +74,25 @@ static bool gen_rp_test_migrate_msix(void *opaque, int version_id)\n> return rp->migrate_msix;\n> }\n> \n> +static void gen_rp_realize(DeviceState *d, Error **errp)\n> +{\n> + GenPCIERootPortClass *grpc = GEN_PCIE_ROOT_PORT_GET_CLASS(d);\n> + GenPCIERootPort *grp = GEN_PCIE_ROOT_PORT(d);\n> + PCIDevice *pci_dev = PCI_DEVICE(d);\n> +\n> + grpc->parent_realize(DEVICE(d), errp);\n> + if (*errp) {\n> + return;\n> + }\n> +\n> + if (!grp->enable_io_fwd) {\n> + pci_word_test_and_clear_mask(pci_dev->wmask + PCI_COMMAND,\n> + PCI_COMMAND_IO);\n> + pci_dev->wmask[PCI_IO_BASE] = 0;\n> + pci_dev->wmask[PCI_IO_LIMIT] = 0;\n> + }\n> +}\n> +\n\nThis function exists now, but with different implementation, from\nAleksandr's commit 226263fb5cda (\"hw/pci: add QEMU-specific PCI\ncapability to the Generic PCI Express Root Port\", 2017-08-18).\n\nIt seems that we now have \"grp->io_reserve\" (a numeric quantity, not a\nboolean), from property \"io-reserve\". The default value is -1.\n\nAccording to the documentation added in c1800a162765 (\"docs: update\ndocumentation considering PCIE-PCI bridge\", 2017-08-18), the value -1\nseems to imply, \"If any reservation field is -1 then this kind of\nreservation is not needed and must be ignored by firmware\".\n\nI think we'll need to refine the definition. Once OVMF starts processing\nthis capability, the behavior should be compatible with earlier\nbehavior. Assume that a user sets only \"mem-reserve\" to something\ndifferent from -1, and thus the capability appears. When OVMF sees the\ncapability, it should be able to tell apart:\n\n- no particular hint about IO space, so continue doing whatever has been\ndone all this time (default IO space reservation),\n- do not request IO space reservation at all,\n- a given (positive) size of IO space should be reserved.\n\nSo I think:\n\n(a) the above read-only mask setting should be done based on\n\n (grp->io_reserve == 0)\n\nand the \"enable-io-fwd\" property should be unnecessary,\n\n(b) the \"io-reserve\" property should be set to 0 for 2.11 machine types\nand onward, and to -1 for 2.10 and earlier (for compatibility),\n\n(c) the documentation in \"docs/pcie_pci_bridge.txt\" should be updated to\nsay:\n* (-1) --> default firmware behavior (unspecified)\n* 0 --> do not reserve\n* >0 --> specific reservation requested\n\n(d) pci_bridge_qemu_reserve_cap_init() should be updated accordingly\n(i.e., a conflict exists if both mem_pref_32_reserve and\nmem_pref_64_reserve are *positive*),\n\n\nSecond, when determining the reservations in OVMF, I would like to look\nonly at the capability fields, and not do a read-write-read-write\nquadruplet to the IO base/limit registers. Do you agree?\n\nThanks!\nLaszlo\n\n\n> static const VMStateDescription vmstate_rp_dev = {\n> .name = \"pcie-root-port\",\n> .version_id = 1,\n> @@ -78,6 +111,7 @@ static const VMStateDescription vmstate_rp_dev = {\n> \n> static Property gen_rp_props[] = {\n> DEFINE_PROP_BOOL(\"x-migrate-msix\", GenPCIERootPort, migrate_msix, true),\n> + DEFINE_PROP_BOOL(\"enable-io-fwd\", GenPCIERootPort, enable_io_fwd, false),\n> DEFINE_PROP_END_OF_LIST()\n> };\n> \n> @@ -86,6 +120,7 @@ static void gen_rp_dev_class_init(ObjectClass *klass, void *data)\n> DeviceClass *dc = DEVICE_CLASS(klass);\n> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);\n> PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);\n> + GenPCIERootPortClass *grpc = GEN_PCIE_ROOT_PORT_CLASS(klass);\n> \n> k->vendor_id = PCI_VENDOR_ID_REDHAT;\n> k->device_id = PCI_DEVICE_ID_REDHAT_PCIE_RP;\n> @@ -96,6 +131,8 @@ static void gen_rp_dev_class_init(ObjectClass *klass, void *data)\n> rpc->interrupts_init = gen_rp_interrupts_init;\n> rpc->interrupts_uninit = gen_rp_interrupts_uninit;\n> rpc->aer_offset = GEN_PCIE_ROOT_PORT_AER_OFFSET;\n> + grpc->parent_realize = dc->realize;\n> + dc->realize = gen_rp_realize;\n> }\n> \n> static const TypeInfo gen_rp_dev_info = {\n> @@ -103,6 +140,7 @@ static const TypeInfo gen_rp_dev_info = {\n> .parent = TYPE_PCIE_ROOT_PORT,\n> .instance_size = sizeof(GenPCIERootPort),\n> .class_init = gen_rp_dev_class_init,\n> + .class_size = sizeof(GenPCIERootPortClass),\n> };\n> \n> static void gen_rp_register_types(void)\n> diff --git a/include/hw/compat.h b/include/hw/compat.h\n> index 3e101f8f67..843bf4a3a5 100644\n> --- a/include/hw/compat.h\n> +++ b/include/hw/compat.h\n> @@ -2,7 +2,11 @@\n> #define HW_COMPAT_H\n> \n> #define HW_COMPAT_2_10 \\\n> - /* empty */\n> + {\\\n> + .driver = \"pcie-root-port\",\\\n> + .property = \"enable-io-fwd\",\\\n> + .value = \"true\",\\\n> + },\n> \n> #define HW_COMPAT_2_9 \\\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-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=lersek@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 3xxcf52wqFz9sMN\n\tfor ;\n\tWed, 20 Sep 2017 08:16:10 +1000 (AEST)","from localhost ([::1]:45691 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1duQoI-00024V-BD\n\tfor incoming@patchwork.ozlabs.org; Tue, 19 Sep 2017 18:16:06 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:56984)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1duQo0-00024L-Au\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 18:15:49 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1duQnw-0006cd-VT\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 18:15:48 -0400","from mx1.redhat.com ([209.132.183.28]:55870)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1duQnw-0006af-LZ\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 18:15:44 -0400","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\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 38258356D4;\n\tTue, 19 Sep 2017 22:15:43 +0000 (UTC)","from lacos-laptop-7.usersys.redhat.com (ovpn-120-37.rdu2.redhat.com\n\t[10.10.120.37])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 39AFA6061E;\n\tTue, 19 Sep 2017 22:15:37 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 38258356D4","To":"Marcel Apfelbaum , qemu-devel@nongnu.org","References":"<20170906142658.58298-1-marcel@redhat.com>\n\t<20170906142658.58298-3-marcel@redhat.com>","From":"Laszlo Ersek ","Message-ID":"<4ca77991-ad6a-a138-622b-b42f410fa1eb@redhat.com>","Date":"Wed, 20 Sep 2017 00:15:36 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170906142658.58298-3-marcel@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.30]);\n\tTue, 19 Sep 2017 22:15:43 +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 2/2] hw/pcie: disable IO port fwd by\n\tdefault for pcie-root-port","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":"pbonzini@redhat.com, mst@redhat.com, ehabkost@redhat.com,\n\t\"Dr. David Alan Gilbert\" , rth@twiddle.net","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1771648,"web_url":"http://patchwork.ozlabs.org/comment/1771648/","msgid":"","list_archive_url":null,"date":"2017-09-20T07:42:46","subject":"Re: [Qemu-devel] [PATCH 2/2] hw/pcie: disable IO port fwd by\n\tdefault for pcie-root-port","submitter":{"id":65362,"url":"http://patchwork.ozlabs.org/api/people/65362/","name":"Marcel Apfelbaum","email":"marcel@redhat.com"},"content":"Hi Laszlo,\n\nOn 20/09/2017 1:15, Laszlo Ersek wrote:\n> Hi Marcel,\n> \n> On 09/06/17 16:26, Marcel Apfelbaum wrote:\n>> For most cases the devices attached to PCIe Root Ports\n>> do not need IO ports range, add an 'enable-io-fwd' property\n>> making it false by default, but keeping it true for older machines.\n>>\n>> Signed-off-by: Marcel Apfelbaum \n>> ---\n>> hw/pci-bridge/gen_pcie_root_port.c | 38 ++++++++++++++++++++++++++++++++++++++\n>> include/hw/compat.h | 6 +++++-\n>> 2 files changed, 43 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c\n>> index cb694d6da5..cbdeb73e2c 100644\n>> --- a/hw/pci-bridge/gen_pcie_root_port.c\n>> +++ b/hw/pci-bridge/gen_pcie_root_port.c\n>> @@ -20,12 +20,26 @@\n>> #define GEN_PCIE_ROOT_PORT_AER_OFFSET 0x100\n>> #define GEN_PCIE_ROOT_PORT_MSIX_NR_VECTOR 1\n>> \n>> +#define GEN_PCIE_ROOT_PORT(obj) \\\n>> + OBJECT_CHECK(GenPCIERootPort, (obj), TYPE_GEN_PCIE_ROOT_PORT)\n>> +#define GEN_PCIE_ROOT_PORT_CLASS(klass) \\\n>> + OBJECT_CLASS_CHECK(GenPCIERootPortClass, (klass), TYPE_GEN_PCIE_ROOT_PORT)\n>> +#define GEN_PCIE_ROOT_PORT_GET_CLASS(obj) \\\n>> + OBJECT_GET_CLASS(GenPCIERootPortClass, (obj), TYPE_GEN_PCIE_ROOT_PORT)\n>> +\n>> +typedef struct GenPCIERootPortClass {\n>> + PCIERootPortClass parent_class;\n>> +\n>> + DeviceRealize parent_realize;\n>> +} GenPCIERootPortClass;\n>> +\n>> typedef struct GenPCIERootPort {\n>> /*< private >*/\n>> PCIESlot parent_obj;\n>> /*< public >*/\n>> \n>> bool migrate_msix;\n>> + bool enable_io_fwd;\n>> } GenPCIERootPort;\n>> \n>> static uint8_t gen_rp_aer_vector(const PCIDevice *d)\n>> @@ -60,6 +74,25 @@ static bool gen_rp_test_migrate_msix(void *opaque, int version_id)\n>> return rp->migrate_msix;\n>> }\n>> \n>> +static void gen_rp_realize(DeviceState *d, Error **errp)\n>> +{\n>> + GenPCIERootPortClass *grpc = GEN_PCIE_ROOT_PORT_GET_CLASS(d);\n>> + GenPCIERootPort *grp = GEN_PCIE_ROOT_PORT(d);\n>> + PCIDevice *pci_dev = PCI_DEVICE(d);\n>> +\n>> + grpc->parent_realize(DEVICE(d), errp);\n>> + if (*errp) {\n>> + return;\n>> + }\n>> +\n>> + if (!grp->enable_io_fwd) {\n>> + pci_word_test_and_clear_mask(pci_dev->wmask + PCI_COMMAND,\n>> + PCI_COMMAND_IO);\n>> + pci_dev->wmask[PCI_IO_BASE] = 0;\n>> + pci_dev->wmask[PCI_IO_LIMIT] = 0;\n>> + }\n>> +}\n>> +\n> \n\nThanks for the review!\n\n> This function exists now, but with different implementation, from\n> Aleksandr's commit 226263fb5cda (\"hw/pci: add QEMU-specific PCI\n> capability to the Generic PCI Express Root Port\", 2017-08-18).\n> \n\nRight, the patches touch the same areas.\n\n> It seems that we now have \"grp->io_reserve\" (a numeric quantity, not a\n> boolean), from property \"io-reserve\". The default value is -1.\n> \n> According to the documentation added in c1800a162765 (\"docs: update\n> documentation considering PCIE-PCI bridge\", 2017-08-18), the value -1\n> seems to imply, \"If any reservation field is -1 then this kind of\n> reservation is not needed and must be ignored by firmware\".\n> \n\nThis was decided after long long upstream discussions with\ndifferent maintainers.\n\n> I think we'll need to refine the definition. Once OVMF starts processing\n> this capability, the behavior should be compatible with earlier\n> behavior. Assume that a user sets only \"mem-reserve\" to something\n> different from -1, and thus the capability appears. When OVMF sees the\n> capability, it should be able to tell apart:\n> \n> - no particular hint about IO space, so continue doing whatever has been\n> done all this time (default IO space reservation),\n> - do not request IO space reservation at all, > - a given (positive) size of IO space should be reserved.\n> \n> So I think:\n> \n> (a) the above read-only mask setting should be done based on\n> \n> (grp->io_reserve == 0)\n> \n> and the \"enable-io-fwd\" property should be unnecessary,\n> \n\nI really want this to be the case, but I need to check the\nimplementation first. The only concern is the semantics.\n\n(grp->io_reserve == 0) hints the firmware to take\nmax(hint, ), since we don't want to reserve less\nthan the IO range needed by existing devices behind the bridge.\n\nThe implementation issue might be that Guest Firmware would\ntake the alignment as default value for an empty root port.\n(maybe take it as a bug and \"solve\" it?)\n\n> (b) the \"io-reserve\" property should be set to 0 for 2.11 machine types\n> and onward, and to -1 for 2.10 and earlier (for compatibility),\n> \n\nMichael asked us to wait a little before changing the default,\nyou can ask him for more info :)\n\n> (c) the documentation in \"docs/pcie_pci_bridge.txt\" should be updated to\n> say:\n> * (-1) --> default firmware behavior (unspecified)\n> * 0 --> do not reserve\n> * >0 --> specific reservation requested\n> \n\nAgreed, once I re-check SeaBIOS to confirm behavior.\n\n> (d) pci_bridge_qemu_reserve_cap_init() should be updated accordingly\n> (i.e., a conflict exists if both mem_pref_32_reserve and\n> mem_pref_64_reserve are *positive*),\n> \n\nI thought we have this check already.\n\n> \n> Second, when determining the reservations in OVMF, I would like to look\n> only at the capability fields, and not do a read-write-read-write\n> quadruplet to the IO base/limit registers. Do you agree?\n> \n\nWell, as stated before, the semantics for the hint is\nmax(hint, ).\nIf you can follow it, that would be OK.\n\n\nGetting back to this patch, setting io-reserve to 0\nwould require also:\n\n + pci_word_test_and_clear_mask(pci_dev->wmask + PCI_COMMAND,\n + PCI_COMMAND_IO);\n + pci_dev->wmask[PCI_IO_BASE] = 0;\n + pci_dev->wmask[PCI_IO_LIMIT] = 0;\n\notherwise the Guest OS would not behave, just be aware of it.\n\n\nThanks,\nMarcel\n\n> Thanks!\n> Laszlo\n> \n> \n>> static const VMStateDescription vmstate_rp_dev = {\n>> .name = \"pcie-root-port\",\n>> .version_id = 1,\n>> @@ -78,6 +111,7 @@ static const VMStateDescription vmstate_rp_dev = {\n>> \n>> static Property gen_rp_props[] = {\n>> DEFINE_PROP_BOOL(\"x-migrate-msix\", GenPCIERootPort, migrate_msix, true),\n>> + DEFINE_PROP_BOOL(\"enable-io-fwd\", GenPCIERootPort, enable_io_fwd, false),\n>> DEFINE_PROP_END_OF_LIST()\n>> };\n>> \n>> @@ -86,6 +120,7 @@ static void gen_rp_dev_class_init(ObjectClass *klass, void *data)\n>> DeviceClass *dc = DEVICE_CLASS(klass);\n>> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);\n>> PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);\n>> + GenPCIERootPortClass *grpc = GEN_PCIE_ROOT_PORT_CLASS(klass);\n>> \n>> k->vendor_id = PCI_VENDOR_ID_REDHAT;\n>> k->device_id = PCI_DEVICE_ID_REDHAT_PCIE_RP;\n>> @@ -96,6 +131,8 @@ static void gen_rp_dev_class_init(ObjectClass *klass, void *data)\n>> rpc->interrupts_init = gen_rp_interrupts_init;\n>> rpc->interrupts_uninit = gen_rp_interrupts_uninit;\n>> rpc->aer_offset = GEN_PCIE_ROOT_PORT_AER_OFFSET;\n>> + grpc->parent_realize = dc->realize;\n>> + dc->realize = gen_rp_realize;\n>> }\n>> \n>> static const TypeInfo gen_rp_dev_info = {\n>> @@ -103,6 +140,7 @@ static const TypeInfo gen_rp_dev_info = {\n>> .parent = TYPE_PCIE_ROOT_PORT,\n>> .instance_size = sizeof(GenPCIERootPort),\n>> .class_init = gen_rp_dev_class_init,\n>> + .class_size = sizeof(GenPCIERootPortClass),\n>> };\n>> \n>> static void gen_rp_register_types(void)\n>> diff --git a/include/hw/compat.h b/include/hw/compat.h\n>> index 3e101f8f67..843bf4a3a5 100644\n>> --- a/include/hw/compat.h\n>> +++ b/include/hw/compat.h\n>> @@ -2,7 +2,11 @@\n>> #define HW_COMPAT_H\n>> \n>> #define HW_COMPAT_2_10 \\\n>> - /* empty */\n>> + {\\\n>> + .driver = \"pcie-root-port\",\\\n>> + .property = \"enable-io-fwd\",\\\n>> + .value = \"true\",\\\n>> + },\n>> \n>> #define HW_COMPAT_2_9 \\\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-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=marcel@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 3xxsFl5Jvxz9s81\n\tfor ;\n\tWed, 20 Sep 2017 17:44:27 +1000 (AEST)","from localhost ([::1]:47240 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1duZgH-0002T4-TW\n\tfor incoming@patchwork.ozlabs.org; Wed, 20 Sep 2017 03:44:25 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:57366)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1duZer-0001uj-8H\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 03:42:58 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1duZen-0003t0-3n\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 03:42:57 -0400","from mx1.redhat.com ([209.132.183.28]:44252)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1duZem-0003nT-H4\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 03:42:52 -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 E48CE356D8;\n\tWed, 20 Sep 2017 07:42:50 +0000 (UTC)","from mapfelba-osx.local (unknown [10.35.206.18])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 602BC5D979;\n\tWed, 20 Sep 2017 07:42:45 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com E48CE356D8","To":"Laszlo Ersek , qemu-devel@nongnu.org","References":"<20170906142658.58298-1-marcel@redhat.com>\n\t<20170906142658.58298-3-marcel@redhat.com>\n\t<4ca77991-ad6a-a138-622b-b42f410fa1eb@redhat.com>","From":"Marcel Apfelbaum ","Message-ID":"","Date":"Wed, 20 Sep 2017 10:42:46 +0300","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":"<4ca77991-ad6a-a138-622b-b42f410fa1eb@redhat.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","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\tWed, 20 Sep 2017 07:42:51 +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 2/2] hw/pcie: disable IO port fwd by\n\tdefault for pcie-root-port","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":"pbonzini@redhat.com, mst@redhat.com, ehabkost@redhat.com,\n\t\"Dr. David Alan Gilbert\" , rth@twiddle.net","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1771903,"web_url":"http://patchwork.ozlabs.org/comment/1771903/","msgid":"<24ca79b9-9350-4339-960b-0dbdf9cd4c5b@redhat.com>","list_archive_url":null,"date":"2017-09-20T11:01:23","subject":"Re: [Qemu-devel] [PATCH 2/2] hw/pcie: disable IO port fwd by\n\tdefault for pcie-root-port","submitter":{"id":9243,"url":"http://patchwork.ozlabs.org/api/people/9243/","name":"Laszlo Ersek","email":"lersek@redhat.com"},"content":"On 09/20/17 09:42, Marcel Apfelbaum wrote:\n> On 20/09/2017 1:15, Laszlo Ersek wrote:\n\n>> It seems that we now have \"grp->io_reserve\" (a numeric quantity, not a\n>> boolean), from property \"io-reserve\". The default value is -1.\n>>\n>> According to the documentation added in c1800a162765 (\"docs: update\n>> documentation considering PCIE-PCI bridge\", 2017-08-18), the value -1\n>> seems to imply, \"If any reservation field is -1 then this kind of\n>> reservation is not needed and must be ignored by firmware\".\n>>\n> \n> This was decided after long long upstream discussions with\n> different maintainers.\n\nI know :) I participated in the review, and the documentation patch\nnoted above carries my R-b.\n\nI didn't mean that the -1 default was wrong, only that we might have to\npolish the definition / semantics a bit.\n\n>> I think we'll need to refine the definition. Once OVMF starts processing\n>> this capability, the behavior should be compatible with earlier\n>> behavior. Assume that a user sets only \"mem-reserve\" to something\n>> different from -1, and thus the capability appears. When OVMF sees the\n>> capability, it should be able to tell apart:\n>>\n>> - no particular hint about IO space, so continue doing whatever has been\n>> done all this time (default IO space reservation),\n>> - do not request IO space reservation at all, > - a given (positive)\n>> size of IO space should be reserved.\n>>\n>> So I think:\n>>\n>> (a) the above read-only mask setting should be done based on\n>>\n>>    (grp->io_reserve == 0)\n>>\n>> and the \"enable-io-fwd\" property should be unnecessary,\n>>\n> \n> I really want this to be the case,  but I need to check the\n> implementation first. The only concern is the semantics.\n> \n> (grp->io_reserve == 0) hints the firmware to take\n> max(hint, ), since we don't want to reserve less\n> than the IO range needed by existing devices behind the bridge.\n\nI agree.\n\nIn OVMF, I would translate (io_reserve == 0) into *not* returning any\nparticular IO reservation hint to the generic PciBusDxe driver, for the\nPCI-E port in question. In turn PciBusDxe follows the above \"max\"\nsemantics, as far as I remember; in other words, setting io_reserve to\nzero wouldn't break existing IO requirements.\n\nHere's a table:\n\n- io_reserve == (-1) --> OVMF returns an IO reservation of 512 bytes\n (which in practice gets rounded up to 4KB). If more IO is needed by\n devices behind the bridge, then that larger value is used. This is the\n current behavior and we should keep it.\n\n- io_reserve == 0 --> OVMF returns no IO reservation at all. If any IO\n is needed by devices behind the bridge, then that value is used.\n Otherwise, no IO is allocated.\n\n- io_reserve > 0 --> OVMF returns this value to PciBusDxe. PciBusDxe\n might round it up. If more IO is needed by devices behind the bridge,\n then that larger value is used.\n\n> The implementation issue might be that Guest Firmware would\n> take the alignment as default value for an empty root port.\n> (maybe take it as a bug and \"solve\" it?)\n\nHm, I don't get this. What do you mean?\n\n> \n>> (b) the \"io-reserve\" property should be set to 0 for 2.11 machine types\n>> and onward, and to -1 for 2.10 and earlier (for compatibility),\n>>\n> \n> Michael asked us to wait a little before changing the default,\n> you can ask him for more info :)\n\nSure, I'm totally fine with delaying the change to the default value.\n\n> \n>> (c) the documentation in \"docs/pcie_pci_bridge.txt\" should be updated to\n>> say:\n>> * (-1) --> default firmware behavior (unspecified)\n>> *   0  --> do not reserve\n>> *  >0  --> specific reservation requested\n>>\n> \n> Agreed, once I re-check SeaBIOS to confirm behavior.\n\nBTW, if the OVMF -- more precisely, PciBusDxe -- and SeaBIOS behaviors\nturn out to differ significantly in the handling of the values (although\nI don't expect it, see above -- last time I looked, the interpretations\nwere similar), I think it's a possibility (although not optimal) to say\nthat the interpretation of these values is firmware specific.\n\nWe won't know for certain until we write the code and test both firmwares.\n\n> \n>> (d) pci_bridge_qemu_reserve_cap_init() should be updated accordingly\n>> (i.e., a conflict exists if both mem_pref_32_reserve and\n>> mem_pref_64_reserve are *positive*),\n>>\n> \n> I thought we have this check already.\n\nWe have a check like this, but it doesn't use the exact condition that\nI'm suggesting above (emphasis on *positive*). We now have\n\n if (mem_pref_32_reserve != (uint32_t)-1 &&\n mem_pref_64_reserve != (uint64_t)-1) {\n error_setg(errp,\n\nbut after introducing the zero value, this is going to be too strict.\nConsider all the possible combinations:\n\n* (-1; -1): default fw behavior, check is OK\n\n* (-1; 0): fw sticks with the default for the first component, picks\n \"no reservation\" for the second component, check is OK\n\n* (-1; >0): fw can treat this identically to ( 0; >0) -- see below. This\n is justified because, while the first component says \"use\n the default\", the entire situation of having such a\n capability is new, so the firmware can introduce new ways\n for handling it. The check passes, which is good.\n\n* ( 0; 0): check reports error, but the firmware can handle this case\n fine (no reservation for either component)\n\n* ( 0; >0): check reports error, but the fw can handle this (no\n reservation for the first component, specific reservation\n for the second)\n\n* (>0; >0): conflict, check is OK\n\n(I'm skipping the description of the inverted orderings, such as (0;-1),\nthey behave similarly.)\n\nSo we need to accept 0 as \"valid\" for either component:\n\n if (mem_pref_32_reserve > 0 &&\n mem_pref_32_reserve < (uint32_t)-1 &&\n mem_pref_64_reserve > 0 &&\n mem_pref_64_reserve < (uint64_t)-1) {\n error_setg(errp,\n\n> \n>>\n>> Second, when determining the reservations in OVMF, I would like to look\n>> only at the capability fields, and not do a read-write-read-write\n>> quadruplet to the IO base/limit registers. Do you agree?\n>>\n> \n> Well, as stated before, the semantics for the hint is\n> max(hint, ).\n> If you can follow it, that would be OK.\n\nIn OVMF there are two separate questions:\n- how to report the requested reservations,\n- how to act upon the reported values.\n\nThe first question pertains to \"platform code\", i.e., what I'm going to\nimplement under OvmfPkg. The second question pertains to \"universal\ncode\" (under \"MdeModulePkg/Bus/Pci/PciBusDxe\"), which is a lot harder to\nmodify, because it is shipped on a wide range of physical platforms too.\n\nAgain, my understanding is that PciBusDxe implements the \"max\" semantics\nthat you describe (pertaining to the 2nd question).\n\nMy interest is in figuring out the 1st question now, that is, where I\nshould take the information from that goes into the \"reservation\ndescription\" that PciBusDxe understands. My preference would be to look\nonly at the PCIE port in question (i.e. no other PCI devices at all),\nand only at said capability in the config space of the PCIE port.\n\n> Getting back to this patch, setting io-reserve to 0\n> would require also:\n> \n>    +      pci_word_test_and_clear_mask(pci_dev->wmask + PCI_COMMAND,\n>    +                                     PCI_COMMAND_IO);\n>    +      pci_dev->wmask[PCI_IO_BASE] = 0;\n>    +      pci_dev->wmask[PCI_IO_LIMIT] = 0;\n> \n> otherwise the Guest OS would not behave, just be aware of it.\n\nAbsolutely, no doubt about this.\n\nThanks,\nLaszlo\n\n\n>>>   static const VMStateDescription vmstate_rp_dev = {\n>>>       .name = \"pcie-root-port\",\n>>>       .version_id = 1,\n>>> @@ -78,6 +111,7 @@ static const VMStateDescription vmstate_rp_dev = {\n>>>     static Property gen_rp_props[] = {\n>>>       DEFINE_PROP_BOOL(\"x-migrate-msix\", GenPCIERootPort,\n>>> migrate_msix, true),\n>>> +    DEFINE_PROP_BOOL(\"enable-io-fwd\", GenPCIERootPort,\n>>> enable_io_fwd, false),\n>>>       DEFINE_PROP_END_OF_LIST()\n>>>   };\n>>>   @@ -86,6 +120,7 @@ static void gen_rp_dev_class_init(ObjectClass\n>>> *klass, void *data)\n>>>       DeviceClass *dc = DEVICE_CLASS(klass);\n>>>       PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);\n>>>       PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);\n>>> +    GenPCIERootPortClass *grpc = GEN_PCIE_ROOT_PORT_CLASS(klass);\n>>>         k->vendor_id = PCI_VENDOR_ID_REDHAT;\n>>>       k->device_id = PCI_DEVICE_ID_REDHAT_PCIE_RP;\n>>> @@ -96,6 +131,8 @@ static void gen_rp_dev_class_init(ObjectClass\n>>> *klass, void *data)\n>>>       rpc->interrupts_init = gen_rp_interrupts_init;\n>>>       rpc->interrupts_uninit = gen_rp_interrupts_uninit;\n>>>       rpc->aer_offset = GEN_PCIE_ROOT_PORT_AER_OFFSET;\n>>> +    grpc->parent_realize = dc->realize;\n>>> +    dc->realize = gen_rp_realize;\n>>>   }\n>>>     static const TypeInfo gen_rp_dev_info = {\n>>> @@ -103,6 +140,7 @@ static const TypeInfo gen_rp_dev_info = {\n>>>       .parent        = TYPE_PCIE_ROOT_PORT,\n>>>       .instance_size = sizeof(GenPCIERootPort),\n>>>       .class_init    = gen_rp_dev_class_init,\n>>> +    .class_size    = sizeof(GenPCIERootPortClass),\n>>>   };\n>>>      static void gen_rp_register_types(void)\n>>> diff --git a/include/hw/compat.h b/include/hw/compat.h\n>>> index 3e101f8f67..843bf4a3a5 100644\n>>> --- a/include/hw/compat.h\n>>> +++ b/include/hw/compat.h\n>>> @@ -2,7 +2,11 @@\n>>>   #define HW_COMPAT_H\n>>>     #define HW_COMPAT_2_10 \\\n>>> -    /* empty */\n>>> +    {\\\n>>> +        .driver   = \"pcie-root-port\",\\\n>>> +        .property = \"enable-io-fwd\",\\\n>>> +        .value    = \"true\",\\\n>>> +    },\n>>>     #define HW_COMPAT_2_9 \\\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-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=lersek@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 3xy1rf2cMdz9sNr\n\tfor ;\n\tThu, 21 Sep 2017 00:11:46 +1000 (AEST)","from localhost ([::1]:48335 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dufj6-00088L-BQ\n\tfor incoming@patchwork.ozlabs.org; Wed, 20 Sep 2017 10:11:44 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:36639)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1duf21-00036x-NN\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 09:27:25 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1duf1h-0004EM-3Q\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 09:27:13 -0400","from mx1.redhat.com ([209.132.183.28]:60436)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1duf1g-0004Dm-Ql\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 09:26:53 -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 EE3077F402;\n\tWed, 20 Sep 2017 11:01:30 +0000 (UTC)","from lacos-laptop-7.usersys.redhat.com (ovpn-120-23.rdu2.redhat.com\n\t[10.10.120.23])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 29E8A60BE2;\n\tWed, 20 Sep 2017 11:01:23 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com EE3077F402","To":"Marcel Apfelbaum , qemu-devel@nongnu.org","References":"<20170906142658.58298-1-marcel@redhat.com>\n\t<20170906142658.58298-3-marcel@redhat.com>\n\t<4ca77991-ad6a-a138-622b-b42f410fa1eb@redhat.com>\n\t","From":"Laszlo Ersek ","Message-ID":"<24ca79b9-9350-4339-960b-0dbdf9cd4c5b@redhat.com>","Date":"Wed, 20 Sep 2017 13:01:23 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","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\tWed, 20 Sep 2017 11:01:31 +0000 (UTC)","Content-Transfer-Encoding":"quoted-printable","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 2/2] hw/pcie: disable IO port fwd by\n\tdefault for pcie-root-port","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":"pbonzini@redhat.com, mst@redhat.com, ehabkost@redhat.com,\n\t\"Dr. David Alan Gilbert\" , rth@twiddle.net","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1771940,"web_url":"http://patchwork.ozlabs.org/comment/1771940/","msgid":"<77945ee2-ccfa-becc-eee1-c4b1066303c7@redhat.com>","list_archive_url":null,"date":"2017-09-20T11:16:42","subject":"Re: [Qemu-devel] [PATCH 2/2] hw/pcie: disable IO port fwd by\n\tdefault for pcie-root-port","submitter":{"id":65362,"url":"http://patchwork.ozlabs.org/api/people/65362/","name":"Marcel Apfelbaum","email":"marcel@redhat.com"},"content":"On 20/09/2017 14:01, Laszlo Ersek wrote:\n> On 09/20/17 09:42, Marcel Apfelbaum wrote:\n>> On 20/09/2017 1:15, Laszlo Ersek wrote:\n> \n>>> It seems that we now have \"grp->io_reserve\" (a numeric quantity, not a\n>>> boolean), from property \"io-reserve\". The default value is -1.\n>>>\n>>> According to the documentation added in c1800a162765 (\"docs: update\n>>> documentation considering PCIE-PCI bridge\", 2017-08-18), the value -1\n>>> seems to imply, \"If any reservation field is -1 then this kind of\n>>> reservation is not needed and must be ignored by firmware\".\n>>>\n>>\n>> This was decided after long long upstream discussions with\n>> different maintainers.\n> \n> I know :) I participated in the review, and the documentation patch\n> noted above carries my R-b.\n> \n> I didn't mean that the -1 default was wrong, only that we might have to\n> polish the definition / semantics a bit.\n> \n>>> I think we'll need to refine the definition. Once OVMF starts processing\n>>> this capability, the behavior should be compatible with earlier\n>>> behavior. Assume that a user sets only \"mem-reserve\" to something\n>>> different from -1, and thus the capability appears. When OVMF sees the\n>>> capability, it should be able to tell apart:\n>>>\n>>> - no particular hint about IO space, so continue doing whatever has been\n>>> done all this time (default IO space reservation),\n>>> - do not request IO space reservation at all, > - a given (positive)\n>>> size of IO space should be reserved.\n>>>\n>>> So I think:\n>>>\n>>> (a) the above read-only mask setting should be done based on\n>>>\n>>>    (grp->io_reserve == 0)\n>>>\n>>> and the \"enable-io-fwd\" property should be unnecessary,\n>>>\n>>\n>> I really want this to be the case,  but I need to check the\n>> implementation first. The only concern is the semantics.\n>>\n>> (grp->io_reserve == 0) hints the firmware to take\n>> max(hint, ), since we don't want to reserve less\n>> than the IO range needed by existing devices behind the bridge.\n> \n> I agree.\n> \n> In OVMF, I would translate (io_reserve == 0) into *not* returning any\n> particular IO reservation hint to the generic PciBusDxe driver, for the\n> PCI-E port in question. In turn PciBusDxe follows the above \"max\"\n> semantics, as far as I remember; in other words, setting io_reserve to\n> zero wouldn't break existing IO requirements.\n> \n> Here's a table:\n> \n> - io_reserve == (-1) --> OVMF returns an IO reservation of 512 bytes\n> (which in practice gets rounded up to 4KB). If more IO is needed by\n> devices behind the bridge, then that larger value is used. This is the\n> current behavior and we should keep it.\n> \n> - io_reserve == 0 --> OVMF returns no IO reservation at all. If any IO\n> is needed by devices behind the bridge, then that value is used.\n> Otherwise, no IO is allocated.\n> \n> - io_reserve > 0 --> OVMF returns this value to PciBusDxe. PciBusDxe\n> might round it up. If more IO is needed by devices behind the bridge,\n> then that larger value is used.\n> \n>> The implementation issue might be that Guest Firmware would\n>> take the alignment as default value for an empty root port.\n>> (maybe take it as a bug and \"solve\" it?)\n> \n> Hm, I don't get this. What do you mean?\n> \n\nEmpty PCI Express Root Port with io_reserve = 0 should end with\nno IO ports range, right?\n\nBut there is some code in SeaBIOS that \"aligns\" the IO range to 4K,\nso we may end up with a 4K range even if io_reserve=0.\n\n\n>>\n>>> (b) the \"io-reserve\" property should be set to 0 for 2.11 machine types\n>>> and onward, and to -1 for 2.10 and earlier (for compatibility),\n>>>\n>>\n>> Michael asked us to wait a little before changing the default,\n>> you can ask him for more info :)\n> \n> Sure, I'm totally fine with delaying the change to the default value.\n> \n>>\n>>> (c) the documentation in \"docs/pcie_pci_bridge.txt\" should be updated to\n>>> say:\n>>> * (-1) --> default firmware behavior (unspecified)\n>>> *   0  --> do not reserve\n>>> *  >0  --> specific reservation requested\n>>>\n>>\n>> Agreed, once I re-check SeaBIOS to confirm behavior.\n> \n> BTW, if the OVMF -- more precisely, PciBusDxe -- and SeaBIOS behaviors\n> turn out to differ significantly in the handling of the values (although\n> I don't expect it, see above -- last time I looked, the interpretations\n> were similar), I think it's a possibility (although not optimal) to say\n> that the interpretation of these values is firmware specific.\n> \n> We won't know for certain until we write the code and test both firmwares.\n> \n>>\n>>> (d) pci_bridge_qemu_reserve_cap_init() should be updated accordingly\n>>> (i.e., a conflict exists if both mem_pref_32_reserve and\n>>> mem_pref_64_reserve are *positive*),\n>>>\n>>\n>> I thought we have this check already.\n> \n> We have a check like this, but it doesn't use the exact condition that\n> I'm suggesting above (emphasis on *positive*). We now have\n> \n> if (mem_pref_32_reserve != (uint32_t)-1 &&\n> mem_pref_64_reserve != (uint64_t)-1) {\n> error_setg(errp,\n> \n> but after introducing the zero value, this is going to be too strict.\n> Consider all the possible combinations:\n> \n> * (-1; -1): default fw behavior, check is OK\n> \n> * (-1; 0): fw sticks with the default for the first component, picks\n> \"no reservation\" for the second component, check is OK\n> \n> * (-1; >0): fw can treat this identically to ( 0; >0) -- see below. This\n> is justified because, while the first component says \"use\n> the default\", the entire situation of having such a\n> capability is new, so the firmware can introduce new ways\n> for handling it. The check passes, which is good.\n> \n> * ( 0; 0): check reports error, but the firmware can handle this case\n> fine (no reservation for either component)\n> \n> * ( 0; >0): check reports error, but the fw can handle this (no\n> reservation for the first component, specific reservation\n> for the second)\n> \n> * (>0; >0): conflict, check is OK\n> \n> (I'm skipping the description of the inverted orderings, such as (0;-1),\n> they behave similarly.)\n> \n> So we need to accept 0 as \"valid\" for either component:\n> \n> if (mem_pref_32_reserve > 0 &&\n> mem_pref_32_reserve < (uint32_t)-1 &&\n> mem_pref_64_reserve > 0 &&\n> mem_pref_64_reserve < (uint64_t)-1) {\n> error_setg(errp,\n> \n\nGot it, you are right. How did we end up with something\ncomplicated again :( ?\n(complicated = needs documentation and not straight forward)\n\n>>\n>>>\n>>> Second, when determining the reservations in OVMF, I would like to look\n>>> only at the capability fields, and not do a read-write-read-write\n>>> quadruplet to the IO base/limit registers. Do you agree?\n>>>\n>>\n>> Well, as stated before, the semantics for the hint is\n>> max(hint, ).\n>> If you can follow it, that would be OK.\n> \n> In OVMF there are two separate questions:\n> - how to report the requested reservations,\n> - how to act upon the reported values.\n> \n> The first question pertains to \"platform code\", i.e., what I'm going to\n> implement under OvmfPkg. The second question pertains to \"universal\n> code\" (under \"MdeModulePkg/Bus/Pci/PciBusDxe\"), which is a lot harder to\n> modify, because it is shipped on a wide range of physical platforms too.\n> \n> Again, my understanding is that PciBusDxe implements the \"max\" semantics\n> that you describe (pertaining to the 2nd question).\n> \n> My interest is in figuring out the 1st question now, that is, where I\n> should take the information from that goes into the \"reservation\n> description\" that PciBusDxe understands. My preference would be to look\n> only at the PCIE port in question (i.e. no other PCI devices at all),\n> and only at said capability in the config space of the PCIE port.\n> \n\nSounds OK.\n\nThanks,\nMarcel\n\n>> Getting back to this patch, setting io-reserve to 0\n>> would require also:\n>>\n>>    +      pci_word_test_and_clear_mask(pci_dev->wmask + PCI_COMMAND,\n>>    +                                     PCI_COMMAND_IO);\n>>    +      pci_dev->wmask[PCI_IO_BASE] = 0;\n>>    +      pci_dev->wmask[PCI_IO_LIMIT] = 0;\n>>\n>> otherwise the Guest OS would not behave, just be aware of it.\n> \n> Absolutely, no doubt about this.\n> \n> Thanks,\n> Laszlo\n> \n> \n>>>>   static const VMStateDescription vmstate_rp_dev = {\n>>>>       .name = \"pcie-root-port\",\n>>>>       .version_id = 1,\n>>>> @@ -78,6 +111,7 @@ static const VMStateDescription vmstate_rp_dev = {\n>>>>     static Property gen_rp_props[] = {\n>>>>       DEFINE_PROP_BOOL(\"x-migrate-msix\", GenPCIERootPort,\n>>>> migrate_msix, true),\n>>>> +    DEFINE_PROP_BOOL(\"enable-io-fwd\", GenPCIERootPort,\n>>>> enable_io_fwd, false),\n>>>>       DEFINE_PROP_END_OF_LIST()\n>>>>   };\n>>>>   @@ -86,6 +120,7 @@ static void gen_rp_dev_class_init(ObjectClass\n>>>> *klass, void *data)\n>>>>       DeviceClass *dc = DEVICE_CLASS(klass);\n>>>>       PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);\n>>>>       PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);\n>>>> +    GenPCIERootPortClass *grpc = GEN_PCIE_ROOT_PORT_CLASS(klass);\n>>>>         k->vendor_id = PCI_VENDOR_ID_REDHAT;\n>>>>       k->device_id = PCI_DEVICE_ID_REDHAT_PCIE_RP;\n>>>> @@ -96,6 +131,8 @@ static void gen_rp_dev_class_init(ObjectClass\n>>>> *klass, void *data)\n>>>>       rpc->interrupts_init = gen_rp_interrupts_init;\n>>>>       rpc->interrupts_uninit = gen_rp_interrupts_uninit;\n>>>>       rpc->aer_offset = GEN_PCIE_ROOT_PORT_AER_OFFSET;\n>>>> +    grpc->parent_realize = dc->realize;\n>>>> +    dc->realize = gen_rp_realize;\n>>>>   }\n>>>>     static const TypeInfo gen_rp_dev_info = {\n>>>> @@ -103,6 +140,7 @@ static const TypeInfo gen_rp_dev_info = {\n>>>>       .parent        = TYPE_PCIE_ROOT_PORT,\n>>>>       .instance_size = sizeof(GenPCIERootPort),\n>>>>       .class_init    = gen_rp_dev_class_init,\n>>>> +    .class_size    = sizeof(GenPCIERootPortClass),\n>>>>   };\n>>>>      static void gen_rp_register_types(void)\n>>>> diff --git a/include/hw/compat.h b/include/hw/compat.h\n>>>> index 3e101f8f67..843bf4a3a5 100644\n>>>> --- a/include/hw/compat.h\n>>>> +++ b/include/hw/compat.h\n>>>> @@ -2,7 +2,11 @@\n>>>>   #define HW_COMPAT_H\n>>>>     #define HW_COMPAT_2_10 \\\n>>>> -    /* empty */\n>>>> +    {\\\n>>>> +        .driver   = \"pcie-root-port\",\\\n>>>> +        .property = \"enable-io-fwd\",\\\n>>>> +        .value    = \"true\",\\\n>>>> +    },\n>>>>     #define HW_COMPAT_2_9 \\\n>>>>       {\\\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-mx04.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=marcel@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 3xy2c82vFyz9sP1\n\tfor ;\n\tThu, 21 Sep 2017 00:46:00 +1000 (AEST)","from localhost ([::1]:48576 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dugGE-0006eo-B3\n\tfor incoming@patchwork.ozlabs.org; Wed, 20 Sep 2017 10:45:58 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:47307)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dufKH-0002MN-ST\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 09:46:07 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dufKB-0001Ro-3J\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 09:46:05 -0400","from mx1.redhat.com ([209.132.183.28]:53702)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dufKA-0001RM-R1\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 09:45:59 -0400","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\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 DC7E67EA94;\n\tWed, 20 Sep 2017 11:16:47 +0000 (UTC)","from mapfelba-osx.local (unknown [10.35.206.30])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 807586062E;\n\tWed, 20 Sep 2017 11:16:41 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com DC7E67EA94","To":"Laszlo Ersek , qemu-devel@nongnu.org","References":"<20170906142658.58298-1-marcel@redhat.com>\n\t<20170906142658.58298-3-marcel@redhat.com>\n\t<4ca77991-ad6a-a138-622b-b42f410fa1eb@redhat.com>\n\t\n\t<24ca79b9-9350-4339-960b-0dbdf9cd4c5b@redhat.com>","From":"Marcel Apfelbaum ","Message-ID":"<77945ee2-ccfa-becc-eee1-c4b1066303c7@redhat.com>","Date":"Wed, 20 Sep 2017 14:16:42 +0300","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":"<24ca79b9-9350-4339-960b-0dbdf9cd4c5b@redhat.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.28]);\n\tWed, 20 Sep 2017 11:16:48 +0000 (UTC)","Content-Transfer-Encoding":"quoted-printable","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 2/2] hw/pcie: disable IO port fwd by\n\tdefault for pcie-root-port","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":"pbonzini@redhat.com, mst@redhat.com, ehabkost@redhat.com,\n\t\"Dr. David Alan Gilbert\" , rth@twiddle.net","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1771970,"web_url":"http://patchwork.ozlabs.org/comment/1771970/","msgid":"<3920012f-1af2-acc4-4c41-77327482c008@redhat.com>","list_archive_url":null,"date":"2017-09-20T11:35:43","subject":"Re: [Qemu-devel] [PATCH 2/2] hw/pcie: disable IO port fwd by\n\tdefault for pcie-root-port","submitter":{"id":9243,"url":"http://patchwork.ozlabs.org/api/people/9243/","name":"Laszlo Ersek","email":"lersek@redhat.com"},"content":"On 09/20/17 13:16, Marcel Apfelbaum wrote:\n> On 20/09/2017 14:01, Laszlo Ersek wrote:\n>> On 09/20/17 09:42, Marcel Apfelbaum wrote:\n\n>>> The implementation issue might be that Guest Firmware would\n>>> take the alignment as default value for an empty root port.\n>>> (maybe take it as a bug and \"solve\" it?)\n>>\n>> Hm, I don't get this. What do you mean?\n>>\n> \n> Empty PCI Express Root Port with io_reserve = 0 should end with\n> no IO ports range, right?\n\nYes.\n\n> But there is some code in SeaBIOS that \"aligns\" the IO range to 4K,\n> so we may end up with a 4K range even if io_reserve=0.\n\nAh.\n\nOK, if this makes a difference, please report your findings :)\n\n>>>> (d) pci_bridge_qemu_reserve_cap_init() should be updated accordingly\n>>>> (i.e., a conflict exists if both mem_pref_32_reserve and\n>>>> mem_pref_64_reserve are *positive*),\n>>>>\n>>>\n>>> I thought we have this check already.\n>>\n>> We have a check like this, but it doesn't use the exact condition that\n>> I'm suggesting above (emphasis on *positive*). We now have\n>>\n>>      if (mem_pref_32_reserve != (uint32_t)-1 &&\n>>          mem_pref_64_reserve != (uint64_t)-1) {\n>>          error_setg(errp,\n>>\n>> but after introducing the zero value, this is going to be too strict.\n>> Consider all the possible combinations:\n>>\n>> * (-1; -1): default fw behavior, check is OK\n>>\n>> * (-1;  0): fw sticks with the default for the first component, picks\n>>              \"no reservation\" for the second component, check is OK\n>>\n>> * (-1; >0): fw can treat this identically to ( 0; >0) -- see below. This\n>>              is justified because, while the first component says \"use\n>>              the default\", the entire situation of having such a\n>>              capability is new, so the firmware can introduce new ways\n>>              for handling it. The check passes, which is good.\n>>\n>> * ( 0;  0): check reports error, but the firmware can handle this case\n>>              fine (no reservation for either component)\n>>\n>> * ( 0; >0): check reports error, but the fw can handle this (no\n>>              reservation for the first component, specific reservation\n>>              for the second)\n>>\n>> * (>0; >0): conflict, check is OK\n>>\n>> (I'm skipping the description of the inverted orderings, such as (0;-1),\n>> they behave similarly.)\n>>\n>> So we need to accept 0 as \"valid\" for either component:\n>>\n>>      if (mem_pref_32_reserve > 0 &&\n>>          mem_pref_32_reserve < (uint32_t)-1 &&\n>>          mem_pref_64_reserve > 0 &&\n>>          mem_pref_64_reserve < (uint64_t)-1) {\n>>          error_setg(errp,\n>>\n> \n> Got it, you are right. How did we end up with something\n> complicated again :( ?\n> (complicated = needs documentation and not straight forward)\n\nHaha, are you serious? :)\n\n*Nothing* is simple in QEMU; nothing at all. :) In general, software is\ncomplex because of feature requests, and QEMU caters to a million\ndifferent (and conflicting) use cases, so you get complexity.\n\n(I know this is trivial, but it sure felt good to write down! :) )\n\n>>>> Second, when determining the reservations in OVMF, I would like to look\n>>>> only at the capability fields, and not do a read-write-read-write\n>>>> quadruplet to the IO base/limit registers. Do you agree?\n>>>>\n>>>\n>>> Well, as stated before, the semantics for the hint is\n>>> max(hint, ).\n>>> If you can follow it, that would be OK.\n>>\n>> In OVMF there are two separate questions:\n>> - how to report the requested reservations,\n>> - how to act upon the reported values.\n>>\n>> The first question pertains to \"platform code\", i.e., what I'm going to\n>> implement under OvmfPkg. The second question pertains to \"universal\n>> code\" (under \"MdeModulePkg/Bus/Pci/PciBusDxe\"), which is a lot harder to\n>> modify, because it is shipped on a wide range of physical platforms too.\n>>\n>> Again, my understanding is that PciBusDxe implements the \"max\" semantics\n>> that you describe (pertaining to the 2nd question).\n>>\n>> My interest is in figuring out the 1st question now, that is, where I\n>> should take the information from that goes into the \"reservation\n>> description\" that PciBusDxe understands. My preference would be to look\n>> only at the PCIE port in question (i.e. no other PCI devices at all),\n>> and only at said capability in the config space of the PCIE port.\n>>\n> \n> Sounds OK.\n\nThank you! Looks like I can start writing code for OVMF when I find the\ntime.\n\nThanks!\nLaszlo","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-mx03.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx03.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=lersek@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 3xy3D73TpTz9s4q\n\tfor ;\n\tThu, 21 Sep 2017 01:13:43 +1000 (AEST)","from localhost ([::1]:48752 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dugh3-0001As-Er\n\tfor incoming@patchwork.ozlabs.org; Wed, 20 Sep 2017 11:13:41 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:51854)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dufYp-000723-5F\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:01:13 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dufYj-0007mr-6U\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:01:07 -0400","from mx1.redhat.com ([209.132.183.28]:43030)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dufYi-0007mM-VH\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:01:01 -0400","from smtp.corp.redhat.com\n\t(int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11])\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 071EE7E42B;\n\tWed, 20 Sep 2017 11:35:50 +0000 (UTC)","from lacos-laptop-7.usersys.redhat.com (ovpn-120-23.rdu2.redhat.com\n\t[10.10.120.23])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 543A9600C2;\n\tWed, 20 Sep 2017 11:35:44 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 071EE7E42B","To":"Marcel Apfelbaum , qemu-devel@nongnu.org","References":"<20170906142658.58298-1-marcel@redhat.com>\n\t<20170906142658.58298-3-marcel@redhat.com>\n\t<4ca77991-ad6a-a138-622b-b42f410fa1eb@redhat.com>\n\t\n\t<24ca79b9-9350-4339-960b-0dbdf9cd4c5b@redhat.com>\n\t<77945ee2-ccfa-becc-eee1-c4b1066303c7@redhat.com>","From":"Laszlo Ersek ","Message-ID":"<3920012f-1af2-acc4-4c41-77327482c008@redhat.com>","Date":"Wed, 20 Sep 2017 13:35:43 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<77945ee2-ccfa-becc-eee1-c4b1066303c7@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.11","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.27]);\n\tWed, 20 Sep 2017 11:35:50 +0000 (UTC)","Content-Transfer-Encoding":"quoted-printable","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 2/2] hw/pcie: disable IO port fwd by\n\tdefault for pcie-root-port","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":"pbonzini@redhat.com, mst@redhat.com, ehabkost@redhat.com,\n\t\"Dr. David Alan Gilbert\" , rth@twiddle.net","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1776203,"web_url":"http://patchwork.ozlabs.org/comment/1776203/","msgid":"<13096938-e70f-7a3a-3cd0-4b9d475c2b18@redhat.com>","list_archive_url":null,"date":"2017-09-27T10:06:30","subject":"Re: [Qemu-devel] [PATCH 2/2] hw/pcie: disable IO port fwd by\n\tdefault for pcie-root-port","submitter":{"id":65362,"url":"http://patchwork.ozlabs.org/api/people/65362/","name":"Marcel Apfelbaum","email":"marcel@redhat.com"},"content":"Hi Laszlo,\n\nOn 20/09/2017 14:01, Laszlo Ersek wrote:\n> We now have\n> \n> if (mem_pref_32_reserve != (uint32_t)-1 &&\n> mem_pref_64_reserve != (uint64_t)-1) {\n> error_setg(errp,\n> \n> but after introducing the zero value, this is going to be too strict.\n> Consider all the possible combinations:\n> \n> * (-1; -1): default fw behavior, check is OK\n> \n> * (-1; 0): fw sticks with the default for the first component, picks\n> \"no reservation\" for the second component, check is OK\n> \n> * (-1; >0): fw can treat this identically to ( 0; >0) -- see below. This\n> is justified because, while the first component says \"use\n> the default\", the entire situation of having such a\n> capability is new, so the firmware can introduce new ways\n> for handling it. The check passes, which is good.\n> \n> * ( 0; 0): check reports error, but the firmware can handle this case\n> fine (no reservation for either component)\n> \n> * ( 0; >0): check reports error, but the fw can handle this (no\n> reservation for the first component, specific reservation\n> for the second)\n> \n> * (>0; >0): conflict, check is OK\n> \n> (I'm skipping the description of the inverted orderings, such as (0;-1),\n> they behave similarly.)\n> \n> So we need to accept 0 as \"valid\" for either component:\n> \n> if (mem_pref_32_reserve > 0 &&\n> mem_pref_32_reserve < (uint32_t)-1 &&\n> mem_pref_64_reserve > 0 &&\n> mem_pref_64_reserve < (uint64_t)-1) {\n> error_setg(errp,\n> \n\nSeaBIOS will not allow (0,0) values for pref32 and pref64 fields.\nMore than that, 0 values are not being taken in consideration.\n\nWe may say is a bug and fix it. But taking a step back, we need the\nhints to reserve *more* mem range, I am not sure 0 values are \ninteresting for mem reservation. Also is also only a hint,\nthe firmware should decide.\n\nDo we have a concrete scenario where would we want to use (0, non-zero)\nor (non-zero, 0) ? (ask for 32/64 prefetch)\n\nOur current semantic is: \"we give a hint to the Guest Firmware,\nwe don't decide. However, giving hints to both pref32 and pref64\nis wrong\".\n\n\nNot related: Using 0 value to disable IO works just fine!\nNo need for disable-IO-fwd property :)\n\nThanks,\nMarcel","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=marcel@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 3y2D5836Q3z9sPm\n\tfor ;\n\tWed, 27 Sep 2017 20:07:08 +1000 (AEST)","from localhost ([::1]:53825 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dx9FC-0007iR-Gh\n\tfor incoming@patchwork.ozlabs.org; Wed, 27 Sep 2017 06:07:06 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:36665)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dx9Eg-0007h0-A5\n\tfor qemu-devel@nongnu.org; Wed, 27 Sep 2017 06:06:35 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dx9Ec-0005Wq-B3\n\tfor qemu-devel@nongnu.org; Wed, 27 Sep 2017 06:06:34 -0400","from mx1.redhat.com ([209.132.183.28]:40762)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dx9Ec-0005Vc-2B\n\tfor qemu-devel@nongnu.org; Wed, 27 Sep 2017 06:06:30 -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 A97A6356EC;\n\tWed, 27 Sep 2017 10:06:27 +0000 (UTC)","from mapfelba-osx.local (unknown [10.35.206.43])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id D74B57A31A;\n\tWed, 27 Sep 2017 10:06:22 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com A97A6356EC","To":"Laszlo Ersek , qemu-devel@nongnu.org","References":"<20170906142658.58298-1-marcel@redhat.com>\n\t<20170906142658.58298-3-marcel@redhat.com>\n\t<4ca77991-ad6a-a138-622b-b42f410fa1eb@redhat.com>\n\t\n\t<24ca79b9-9350-4339-960b-0dbdf9cd4c5b@redhat.com>","From":"Marcel Apfelbaum ","Message-ID":"<13096938-e70f-7a3a-3cd0-4b9d475c2b18@redhat.com>","Date":"Wed, 27 Sep 2017 13:06:30 +0300","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":"<24ca79b9-9350-4339-960b-0dbdf9cd4c5b@redhat.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","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.30]);\n\tWed, 27 Sep 2017 10:06:27 +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 2/2] hw/pcie: disable IO port fwd by\n\tdefault for pcie-root-port","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":"ehabkost@redhat.com, mst@redhat.com,\n\tAlexander Bezzubikov ,\n\t\"Dr. David Alan Gilbert\" ,\n\tpbonzini@redhat.com, rth@twiddle.net","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1776833,"web_url":"http://patchwork.ozlabs.org/comment/1776833/","msgid":"<6e81f338-1a43-518c-b39e-f09de76c3106@redhat.com>","list_archive_url":null,"date":"2017-09-28T07:48:02","subject":"Re: [Qemu-devel] [PATCH 2/2] hw/pcie: disable IO port fwd by\n\tdefault for pcie-root-port","submitter":{"id":9243,"url":"http://patchwork.ozlabs.org/api/people/9243/","name":"Laszlo Ersek","email":"lersek@redhat.com"},"content":"On 09/27/17 12:06, Marcel Apfelbaum wrote:\n> Hi Laszlo,\n> \n> On 20/09/2017 14:01, Laszlo Ersek wrote:\n>>   We now have\n>>\n>>      if (mem_pref_32_reserve != (uint32_t)-1 &&\n>>          mem_pref_64_reserve != (uint64_t)-1) {\n>>          error_setg(errp,\n>>\n>> but after introducing the zero value, this is going to be too strict.\n>> Consider all the possible combinations:\n>>\n>> * (-1; -1): default fw behavior, check is OK\n>>\n>> * (-1;  0): fw sticks with the default for the first component, picks\n>>              \"no reservation\" for the second component, check is OK\n>>\n>> * (-1; >0): fw can treat this identically to ( 0; >0) -- see below. This\n>>              is justified because, while the first component says \"use\n>>              the default\", the entire situation of having such a\n>>              capability is new, so the firmware can introduce new ways\n>>              for handling it. The check passes, which is good.\n>>\n>> * ( 0;  0): check reports error, but the firmware can handle this case\n>>              fine (no reservation for either component)\n>>\n>> * ( 0; >0): check reports error, but the fw can handle this (no\n>>              reservation for the first component, specific reservation\n>>              for the second)\n>>\n>> * (>0; >0): conflict, check is OK\n>>\n>> (I'm skipping the description of the inverted orderings, such as (0;-1),\n>> they behave similarly.)\n>>\n>> So we need to accept 0 as \"valid\" for either component:\n>>\n>>      if (mem_pref_32_reserve > 0 &&\n>>          mem_pref_32_reserve < (uint32_t)-1 &&\n>>          mem_pref_64_reserve > 0 &&\n>>          mem_pref_64_reserve < (uint64_t)-1) {\n>>          error_setg(errp,\n>>\n> \n> SeaBIOS  will not allow (0,0) values for pref32 and pref64 fields.\n> More than that, 0 values are not being taken in consideration.\n> \n> We may say is a bug and fix it. But taking a step back, we need the\n> hints to reserve *more* mem range, I am not sure 0 values are\n> interesting for mem reservation. Also is also only a hint,\n> the firmware should decide.\n> \n> Do we have a concrete scenario where would we want to use (0, non-zero)\n> or (non-zero, 0) ? (ask for 32/64 prefetch)\n> \n> Our current semantic is: \"we give a hint to the Guest Firmware,\n> we don't decide. However, giving hints to both pref32 and pref64\n> is wrong\".\n\nAfter having written the OVMF code for this, my understanding is\nclearer, and I agree that, for *prefetchable*, distinguishing -1 from 0\nis not necessary (-1 means \"firmware default (unspecified)\", and 0 means\n\"do not reserve\"; and for prefetchable, the two things mean the same in\nOVMF). So the current QEMU code should be fine.\n\n> Not related: Using 0 value to disable IO works just fine!\n> No need for disable-IO-fwd property :)\n\nGreat!\n\nThanks!\nLaszlo","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-mx09.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx09.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=lersek@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 3y2myw6pfGz9t5x\n\tfor ;\n\tThu, 28 Sep 2017 17:48:39 +1000 (AEST)","from localhost ([::1]:57818 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dxTYh-0005ij-GJ\n\tfor incoming@patchwork.ozlabs.org; Thu, 28 Sep 2017 03:48:35 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:37858)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dxTYN-0005id-CJ\n\tfor qemu-devel@nongnu.org; Thu, 28 Sep 2017 03:48:16 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dxTYK-000128-9A\n\tfor qemu-devel@nongnu.org; Thu, 28 Sep 2017 03:48:15 -0400","from mx1.redhat.com ([209.132.183.28]:45160)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dxTYJ-00011L-WE\n\tfor qemu-devel@nongnu.org; Thu, 28 Sep 2017 03:48:12 -0400","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\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 F2A4C4DF0D0;\n\tThu, 28 Sep 2017 07:48:09 +0000 (UTC)","from lacos-laptop-7.usersys.redhat.com (ovpn-120-73.rdu2.redhat.com\n\t[10.10.120.73])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 03C8D93DDE;\n\tThu, 28 Sep 2017 07:48:03 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com F2A4C4DF0D0","To":"Marcel Apfelbaum , qemu-devel@nongnu.org","References":"<20170906142658.58298-1-marcel@redhat.com>\n\t<20170906142658.58298-3-marcel@redhat.com>\n\t<4ca77991-ad6a-a138-622b-b42f410fa1eb@redhat.com>\n\t\n\t<24ca79b9-9350-4339-960b-0dbdf9cd4c5b@redhat.com>\n\t<13096938-e70f-7a3a-3cd0-4b9d475c2b18@redhat.com>","From":"Laszlo Ersek ","Message-ID":"<6e81f338-1a43-518c-b39e-f09de76c3106@redhat.com>","Date":"Thu, 28 Sep 2017 09:48:02 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<13096938-e70f-7a3a-3cd0-4b9d475c2b18@redhat.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.38]);\n\tThu, 28 Sep 2017 07:48:10 +0000 (UTC)","Content-Transfer-Encoding":"quoted-printable","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 2/2] hw/pcie: disable IO port fwd by\n\tdefault for pcie-root-port","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":"ehabkost@redhat.com, mst@redhat.com,\n\tAlexander Bezzubikov ,\n\t\"Dr. David Alan Gilbert\" ,\n\tpbonzini@redhat.com, rth@twiddle.net","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]