From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 79829C761A6 for ; Fri, 31 Mar 2023 14:29:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:References:Cc:To:Subject:From: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=kB+RhVe0bzDUTzG55l57GX3fbJm5T/Bod89OJsRZQsw=; b=38csedWRwwzN9u 6j+Pg/A44LOK3MThUf1FC5jAQT2D5LkgC2HzXC1X+PMWxHacwBl90VWqnDdUL1afh8i/eGS6SQyMs LogK4JmCXNFgntuXBem8kRtTguLS9/STb+xhTlGPOyhFV9u3b5UEbzSL3x8fwYs82xgGBm8XlEdK9 qkYjVvB84xGtEMhg3F3zuJ2l+dg8Bf5kwfVTUqRj0Leu+H6irxYdIM8kyD2On9FF9nExJh40xUc9h 4+cLSqehpdLKePBKEwey2/zY/oSqWL67JqyFecNN+E8a7pVkuEm0ZtKkpli1yR2Z8G8wFjCfN1Fmp 05AE4xZDbpdOmsUELs9Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1piFjl-007hNO-23; Fri, 31 Mar 2023 14:28:17 +0000 Received: from mail-qt1-x829.google.com ([2607:f8b0:4864:20::829]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1piFiy-007gx9-0Q for linux-arm-kernel@lists.infradead.org; Fri, 31 Mar 2023 14:27:31 +0000 Received: by mail-qt1-x829.google.com with SMTP id cr18so17978758qtb.0 for ; Fri, 31 Mar 2023 07:27:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1680272846; x=1682864846; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:subject:from:user-agent:mime-version:date:message-id:from:to :cc:subject:date:message-id:reply-to; bh=ovU31hB83XMxRuvQsZcOBkvqCc2tCiYd8mVr7X/VGuU=; b=oUoKSDrgPNpD6m7Lqp0V8yC3WuZ3OehDUyirBcPRLk3PZHk1Gn9ZcbE1yBKMRYH2AP fjvGRL+MOhbb6exgBqG4kwc5QULmtV2wXOHkNzglsqZAYQbtML+9/Pw1MaqbdU6PH93t SiXevR6eNZtKWfl884OUU1H35rRVnGG7DdfRWdDeKbZgVHYs6TriHPgTc1nsHpN6guPg sfxsuqNdz1qNLukijvVcBIggaD0+qFnoxZKTFMIYzGQpE2xuf6PDKs7VfYf9nklNXVX9 VC2dyPi+WpHNgGaQXqq1sExi9KYzxLgjTQoH1drzxmOtNivHN13hVUneEAArxpnH4ol9 XApA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680272846; x=1682864846; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:subject:from:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ovU31hB83XMxRuvQsZcOBkvqCc2tCiYd8mVr7X/VGuU=; b=wSeMi86CFwAR9r6fSYcKsdpg1BDiKcMlKbCNXDyCYJda0pcs4HvciT42SjEvV4i2E7 zzr5G8EfIs003hVMaoy6Fv+sV2MzbCqRiw5vYAaGISxU6A1zeuiZsULpsnij6q8jrZxG snQ5agJOeREV0XDByZtMARYXUA0KdPCqUl5bkxMndS8+zTcN2PzPCXjz+eujqRkl65bF 7lPXiuaCN8HVu+0Rz8DUez16RgvTuJIjy8VLIil1PaC78uPADXUkc9KyX8kGxpRpJ+3b uRQp0rcs5q/3rb4BTs+6ztfSkpwxXl52cZbDFdHem1MPP+KEV65TD+RokdmrTpvMzrRc oSRA== X-Gm-Message-State: AAQBX9fkTaeflGqyc5vfzvvxuCB+12iWI+5zsIz39l0JGr10yel+/KT0 +SkEs8hEtLsrQgQexsVKZvChfA== X-Google-Smtp-Source: AKy350ba/HmTG59/0ZwTkuQJMZac0m7+0VObYhtuBg3FOuNR8IRNULKuMY+8+G7Nuj6f89cA6toZAg== X-Received: by 2002:a05:622a:174c:b0:3e6:326c:c904 with SMTP id l12-20020a05622a174c00b003e6326cc904mr9535154qtk.4.1680272845931; Fri, 31 Mar 2023 07:27:25 -0700 (PDT) Received: from [172.22.22.4] ([98.61.227.136]) by smtp.googlemail.com with ESMTPSA id bm20-20020a05620a199400b007435a646354sm711881qkb.0.2023.03.31.07.27.24 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 31 Mar 2023 07:27:25 -0700 (PDT) Message-ID: Date: Fri, 31 Mar 2023 09:27:23 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 From: Alex Elder Subject: Re: [PATCH v11 21/26] virt: gunyah: Add IO handlers To: Elliot Berman , Srinivas Kandagatla , Prakruthi Deepak Heragu Cc: Murali Nalajala , Trilok Soni , Srivatsa Vaddagiri , Carl van Schaik , Dmitry Baryshkov , Bjorn Andersson , Konrad Dybcio , Arnd Bergmann , Greg Kroah-Hartman , Rob Herring , Krzysztof Kozlowski , Jonathan Corbet , Bagas Sanjaya , Will Deacon , Andy Gross , Catalin Marinas , Jassi Brar , linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-arm-kernel@lists.infradead.org References: <20230304010632.2127470-1-quic_eberman@quicinc.com> <20230304010632.2127470-22-quic_eberman@quicinc.com> Content-Language: en-US In-Reply-To: <20230304010632.2127470-22-quic_eberman@quicinc.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230331_072728_176843_73CB6AB4 X-CRM114-Status: GOOD ( 26.88 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 3/3/23 7:06 PM, Elliot Berman wrote: > Add framework for VM functions to handle stage-2 write faults from Gunyah > guest virtual machines. IO handlers have a range of addresses which they > apply to. Optionally, they may apply to only when the value written > matches the IO handler's value. > > Co-developed-by: Prakruthi Deepak Heragu > Signed-off-by: Prakruthi Deepak Heragu > Signed-off-by: Elliot Berman Two (related) bugs and a suggestion that might help avoid adding the same problem in the future. (Or maybe I made that suggestion elsewhere? Anyway, you'll see.) -Alex > --- > drivers/virt/gunyah/vm_mgr.c | 94 +++++++++++++++++++++++++++++++++++ > drivers/virt/gunyah/vm_mgr.h | 4 ++ > include/linux/gunyah_vm_mgr.h | 25 ++++++++++ > 3 files changed, 123 insertions(+) > > diff --git a/drivers/virt/gunyah/vm_mgr.c b/drivers/virt/gunyah/vm_mgr.c > index 0269bcdaf692..b31fac15ff45 100644 > --- a/drivers/virt/gunyah/vm_mgr.c > +++ b/drivers/virt/gunyah/vm_mgr.c > @@ -233,6 +233,100 @@ static void gh_vm_add_resource(struct gh_vm *ghvm, struct gh_resource *ghrsc) > mutex_unlock(&ghvm->resources_lock); > } > > +static int _gh_vm_io_handler_compare(const struct rb_node *node, const struct rb_node *parent) > +{ > + struct gh_vm_io_handler *n = container_of(node, struct gh_vm_io_handler, node); > + struct gh_vm_io_handler *p = container_of(parent, struct gh_vm_io_handler, node); > + > + if (n->addr < p->addr) > + return -1; > + if (n->addr > p->addr) > + return 1; > + if ((n->len && !p->len) || (!n->len && p->len)) > + return 0; > + if (n->len < p->len) > + return -1; > + if (n->len > p->len) > + return 1; The datamatch field in a gh_vm_io_handler structure is Boolean. If this is what you intend, it would be better to not treat them as integer values (i.e., don't use < and >). However I *think* what you want is to be comparing the data fields here. If so, this is a BUG. I think you should maybe use "data" in the gh_fn_ioeventfd_arg structure rather than "datamatch". And then use "datamatch" consistently as a Boolean indicating whether to do matching, and "data" to be the value used in matching. > + if (n->datamatch < p->datamatch) > + return -1; > + if (n->datamatch > p->datamatch) > + return 1; > + return 0; > +} > + > +static int gh_vm_io_handler_compare(struct rb_node *node, const struct rb_node *parent) > +{ > + return _gh_vm_io_handler_compare(node, parent); > +} > + > +static int gh_vm_io_handler_find(const void *key, const struct rb_node *node) > +{ > + const struct gh_vm_io_handler *k = key; > + > + return _gh_vm_io_handler_compare(&k->node, node); > +} > + > +static struct gh_vm_io_handler *gh_vm_mgr_find_io_hdlr(struct gh_vm *ghvm, u64 addr, > + u64 len, u64 data) > +{ > + struct gh_vm_io_handler key = { > + .addr = addr, > + .len = len, > + .datamatch = data, The datamatch field here is Boolean. I'm pretty sure you want to assign the data field instead, in which case, this is a BUG. If you *do* intend to treat the data assigned as Boolean, please use !!data to make this obvious. > + }; > + struct rb_node *node; > + > + node = rb_find(&key, &ghvm->mmio_handler_root, gh_vm_io_handler_find); > + if (!node) > + return NULL; > + > + return container_of(node, struct gh_vm_io_handler, node); > +} > + > +int gh_vm_mmio_write(struct gh_vm *ghvm, u64 addr, u32 len, u64 data) > +{ > + struct gh_vm_io_handler *io_hdlr = NULL; > + int ret; > + > + down_read(&ghvm->mmio_handler_lock); > + io_hdlr = gh_vm_mgr_find_io_hdlr(ghvm, addr, len, data); > + if (!io_hdlr || !io_hdlr->ops || !io_hdlr->ops->write) { > + ret = -ENODEV; > + goto out; > + } > + > + ret = io_hdlr->ops->write(io_hdlr, addr, len, data); > + > +out: > + up_read(&ghvm->mmio_handler_lock); > + return ret; > +} > +EXPORT_SYMBOL_GPL(gh_vm_mmio_write); . . . _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel