From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v3 07/24] xen/arm: Introduce xen, passthrough property Date: Wed, 28 Jan 2015 16:53:28 +0000 Message-ID: <54C91408.2020007@linaro.org> References: <1421159133-31526-1-git-send-email-julien.grall@linaro.org> <1421159133-31526-8-git-send-email-julien.grall@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YGVsM-0003Ub-Gy for xen-devel@lists.xenproject.org; Wed, 28 Jan 2015 16:53:58 +0000 Received: by mail-we0-f182.google.com with SMTP id l61so21924602wev.13 for ; Wed, 28 Jan 2015 08:53:56 -0800 (PST) In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: xen-devel@lists.xenproject.org, tim@xen.org, ian.campbell@citrix.com, stefano.stabellini@citrix.com List-Id: xen-devel@lists.xenproject.org On 28/01/15 16:36, Stefano Stabellini wrote: > On Tue, 13 Jan 2015, Julien Grall wrote: >> When a device is marked for passthrough (via the new property "xen,passthrough"), >> dom0 must not access to the device (i.e not loading a driver), but should >> be able to manage the MMIO/interrupt of the passthrough device. >> >> The latter part will allow the toolstack to map MMIO/IRQ when a device >> is pass through to a guest. >> >> The property "xen,passthrough" will be translated as 'status="disabled"' >> in the device tree to avoid DOM0 using the device. We assume that DOM0 is >> able to cope with this property (already the case for Linux). >> >> Rework the function map_device (renamed into handle_device) to: >> >> * For a given device node: >> - Give permission to manage IRQ/MMIO for this device >> - Retrieve the IRQ configuration (i.e edge/level) from the device >> tree >> * When the device is not marked for guest passthrough: >> - Assign the device to the guest if it's protected by an IOMMU >> - Map the IRQs and MMIOs regions to the guest >> >> Signed-off-by: Julien Grall >> >> --- >> Changes in v3: >> - This patch was formely "xen/arm: Follow-up to allow DOM0 >> manage IRQ and MMIO". It has been split in 2 parts [1]. >> - Update commit title and improve message >> - Remove spurious change >> >> [1] https://patches.linaro.org/34669/ >> --- >> docs/misc/arm/device-tree/passthrough.txt | 7 ++ >> xen/arch/arm/device.c | 2 +- >> xen/arch/arm/domain_build.c | 102 ++++++++++++++++++++++-------- >> xen/common/device_tree.c | 6 ++ >> xen/include/xen/device_tree.h | 11 ++++ >> 5 files changed, 100 insertions(+), 28 deletions(-) >> create mode 100644 docs/misc/arm/device-tree/passthrough.txt >> >> diff --git a/docs/misc/arm/device-tree/passthrough.txt b/docs/misc/arm/device-tree/passthrough.txt >> new file mode 100644 >> index 0000000..04645b3 >> --- /dev/null >> +++ b/docs/misc/arm/device-tree/passthrough.txt >> @@ -0,0 +1,7 @@ >> +Device passthrough >> +=================== >> + >> +Any device that will be passthrough to a guest should have a property >> +"xen,passthrough" in their device tree node. >> + >> +The device won't be exposed to DOM0 and therefore no driver will be loaded. >> diff --git a/xen/arch/arm/device.c b/xen/arch/arm/device.c >> index 1993929..1a01793 100644 >> --- a/xen/arch/arm/device.c >> +++ b/xen/arch/arm/device.c >> @@ -30,7 +30,7 @@ int __init device_init(struct dt_device_node *dev, enum device_match type, >> >> ASSERT(dev != NULL); >> >> - if ( !dt_device_is_available(dev) ) >> + if ( !dt_device_is_available(dev) || dt_device_for_passthrough(dev) ) >> return -ENODEV; >> >> for ( desc = _sdevice; desc != _edevice; desc++ ) >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index f68755f..b48b5d0 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -402,7 +402,7 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo, >> const struct dt_device_node *node) >> { >> const char *bootargs = NULL; >> - const struct dt_property *prop; >> + const struct dt_property *prop, *status = NULL; >> int res = 0; >> int had_dom0_bootargs = 0; >> >> @@ -457,6 +457,17 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo, >> } >> } >> >> + /* Don't expose the property "xen,passthrough" to the guest */ >> + if ( dt_property_name_is_equal(prop, "xen,passthrough") ) >> + continue; >> + >> + /* Remember and skip the status property as Xen may modify it later */ >> + if ( dt_property_name_is_equal(prop, "status") ) >> + { >> + status = prop; >> + continue; >> + } >> + >> res = fdt_property(kinfo->fdt, prop->name, prop_data, prop_len); >> >> xfree(new_data); >> @@ -465,6 +476,19 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo, >> return res; >> } >> >> + /* >> + * Override the property "status" to disable the device when it's >> + * marked for passthrough. >> + */ >> + if ( dt_device_for_passthrough(node) ) >> + res = fdt_property_string(kinfo->fdt, "status", "disabled"); >> + else if ( status ) >> + res = fdt_property(kinfo->fdt, "status", status->value, >> + status->length); > > Why is the "else" needed? Wouldn't the status property for > non-passtrough devices already be covered by the > dt_for_each_property_node loop above? Because the property "status" is skipped earlier in any case. Regards, -- Julien Grall