From patchwork Mon Feb 16 14:50:54 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julien Grall X-Patchwork-Id: 44711 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-we0-f197.google.com (mail-we0-f197.google.com [74.125.82.197]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 92AE321544 for ; Mon, 16 Feb 2015 14:52:54 +0000 (UTC) Received: by mail-we0-f197.google.com with SMTP id w55sf16010230wes.0 for ; Mon, 16 Feb 2015 06:52:53 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:from:to:date:message-id:in-reply-to :references:cc:subject:precedence:list-id:list-unsubscribe:list-post :list-help:list-subscribe:mime-version:content-type :content-transfer-encoding:sender:errors-to:x-original-sender :x-original-authentication-results:mailing-list:list-archive; bh=e74HIMeljtbl4AKhV6w1A9g7KN0RiuULLMwV9gQ+u8w=; b=GjA4oKmG04yH/RrPfwKo7l3Gc6Xeq653OBTVNnlDdJFPV4sv/KB1oUUIyNBEHo2o/l YIftnXKK2nKxIoIHV8LofI56ig5KzXA7HBV4bo28W36gzmKuX3jpLxrC/UKoIBF1BTeu vzCIzxL4Dkn6ks4gz5aZCvQ0YOU6t8pjeHbOyIYsRavBA14g4uLlVbWh8HjHtk49b3Bl 9H5H2FUrWsIX8BrvEKj54hqlufY/dSJGUYTnCVlj0NJkBCJCAzfAarfm51Jz73MUzeWP Fa0WV7yzXLPP64o2VyMywqqUfo1BkDUaQaSvQcFzVZ5rP4Z8u3lHpU0nEgnCq8EsrTsv iN6A== X-Gm-Message-State: ALoCoQnhqB1dKuX7MzyjBncLop9XuE9uLZBp2Ln3qP053J5BRZnoyqyyDnxBPSG1iWzNNfUfoIee X-Received: by 10.112.171.37 with SMTP id ar5mr3028061lbc.16.1424098373906; Mon, 16 Feb 2015 06:52:53 -0800 (PST) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.43.163 with SMTP id x3ls513424lal.96.gmail; Mon, 16 Feb 2015 06:52:53 -0800 (PST) X-Received: by 10.152.6.66 with SMTP id y2mr7265743lay.25.1424098373539; Mon, 16 Feb 2015 06:52:53 -0800 (PST) Received: from mail-la0-f52.google.com (mail-la0-f52.google.com. [209.85.215.52]) by mx.google.com with ESMTPS id po2si8291241lbc.18.2015.02.16.06.52.53 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 16 Feb 2015 06:52:53 -0800 (PST) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.52 as permitted sender) client-ip=209.85.215.52; Received: by labms9 with SMTP id ms9so23332302lab.10 for ; Mon, 16 Feb 2015 06:52:53 -0800 (PST) X-Received: by 10.152.87.3 with SMTP id t3mr21354928laz.19.1424098373423; Mon, 16 Feb 2015 06:52:53 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.35.133 with SMTP id h5csp1629495lbj; Mon, 16 Feb 2015 06:52:52 -0800 (PST) X-Received: by 10.140.22.234 with SMTP id 97mr1107129qgn.21.1424098371663; Mon, 16 Feb 2015 06:52:51 -0800 (PST) Received: from lists.xen.org (lists.xen.org. [50.57.142.19]) by mx.google.com with ESMTPS id h13si1133322qhc.98.2015.02.16.06.52.50 (version=TLSv1 cipher=RC4-SHA bits=128/128); Mon, 16 Feb 2015 06:52:51 -0800 (PST) Received-SPF: none (google.com: xen-devel-bounces@lists.xen.org does not designate permitted sender hosts) client-ip=50.57.142.19; Received: from localhost ([127.0.0.1] helo=lists.xen.org) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YNN1Y-0001sF-Us; Mon, 16 Feb 2015 14:51:48 +0000 Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YNN1X-0001ol-6J for xen-devel@lists.xenproject.org; Mon, 16 Feb 2015 14:51:47 +0000 Received: from [85.158.137.68] by server-1.bemta-3.messagelabs.com id C4/A8-02999-20402E45; Mon, 16 Feb 2015 14:51:46 +0000 X-Env-Sender: julien.grall@linaro.org X-Msg-Ref: server-12.tower-31.messagelabs.com!1424098305!11930609!1 X-Originating-IP: [74.125.82.53] X-SpamReason: No, hits=0.0 required=7.0 tests= X-StarScan-Received: X-StarScan-Version: 6.13.4; banners=-,-,- X-VirusChecked: Checked Received: (qmail 11367 invoked from network); 16 Feb 2015 14:51:45 -0000 Received: from mail-wg0-f53.google.com (HELO mail-wg0-f53.google.com) (74.125.82.53) by server-12.tower-31.messagelabs.com with RC4-SHA encrypted SMTP; 16 Feb 2015 14:51:45 -0000 Received: by mail-wg0-f53.google.com with SMTP id a1so13203804wgh.12 for ; Mon, 16 Feb 2015 06:51:45 -0800 (PST) X-Received: by 10.194.176.33 with SMTP id cf1mr48892352wjc.19.1424098305170; Mon, 16 Feb 2015 06:51:45 -0800 (PST) Received: from chilopoda.uk.xensource.com. ([185.25.64.249]) by mx.google.com with ESMTPSA id dj5sm23172398wjb.28.2015.02.16.06.51.43 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 16 Feb 2015 06:51:44 -0800 (PST) From: Julien Grall To: xen-devel@lists.xenproject.org Date: Mon, 16 Feb 2015 14:50:54 +0000 Message-Id: <1424098255-22490-15-git-send-email-julien.grall@linaro.org> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1424098255-22490-1-git-send-email-julien.grall@linaro.org> References: <1424098255-22490-1-git-send-email-julien.grall@linaro.org> Cc: stefano.stabellini@citrix.com, Vijaya.Kumar@caviumnetworks.com, Julien Grall , tim@xen.org, ian.campbell@citrix.com Subject: [Xen-devel] [PATCH v3 14/15] xen/arm: vgic: Drop iactive, ipend, pendsgi field X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Post: , List-Help: , List-Subscribe: , MIME-Version: 1.0 Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: julien.grall@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.52 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Archive: The current VGIC code doesn't support to change the pending and active status of an IRQ via the (re-)distributor. Futhermore, all the access size wasn't support correctly and some registers was implemented as write-ignore. The latter make very difficult for a kernel developer to find that we don't support R/W to those registers. Make the support consistent: - read will return 0 (RAZ) - write will print an error and inject a data abort to the guest Also, those fields was never set and field such as ipend and pendsgi was doing the same jobs. Rather than wasting memory, we should better drop it. We could re-introduce them if we need it when the support will be made. Signed-off-by: Julien Grall --- Changes in v3: - Re-work commit message Changes in v2: - Patch added --- xen/arch/arm/vgic-v2.c | 71 +++++-------------------- xen/arch/arm/vgic-v3.c | 125 +++++++++++++++------------------------------ xen/include/asm-arm/vgic.h | 2 +- 3 files changed, 55 insertions(+), 143 deletions(-) diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c index 1a02541..3cf67a9 100644 --- a/xen/arch/arm/vgic-v2.c +++ b/xen/arch/arm/vgic-v2.c @@ -94,41 +94,15 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info) vgic_unlock_rank(v, rank, flags); return 1; + /* Read the pending status of an IRQ via GICD is not supported */ case GICD_ISPENDR ... GICD_ISPENDRN: - if ( dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISPENDR, DABT_WORD); - if ( rank == NULL) goto read_as_zero; - vgic_lock_rank(v, rank, flags); - *r = vgic_byte_read(rank->ipend, dabt.sign, gicd_reg); - vgic_unlock_rank(v, rank, flags); - return 1; - case GICD_ICPENDR ... GICD_ICPENDRN: - if ( dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 0, gicd_reg - GICD_ICPENDR, DABT_WORD); - if ( rank == NULL) goto read_as_zero; - vgic_lock_rank(v, rank, flags); - *r = vgic_byte_read(rank->ipend, dabt.sign, gicd_reg); - vgic_unlock_rank(v, rank, flags); - return 1; + goto read_as_zero; + /* Read the active status of an IRQ via GICD is not supported */ case GICD_ISACTIVER ... GICD_ISACTIVERN: - if ( dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISACTIVER, DABT_WORD); - if ( rank == NULL) goto read_as_zero; - vgic_lock_rank(v, rank, flags); - *r = rank->iactive; - vgic_unlock_rank(v, rank, flags); - return 1; - case GICD_ICACTIVER ... GICD_ICACTIVERN: - if ( dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICACTIVER, DABT_WORD); - if ( rank == NULL) goto read_as_zero; - vgic_lock_rank(v, rank, flags); - *r = rank->iactive; - vgic_unlock_rank(v, rank, flags); - return 1; + goto read_as_zero; case GICD_ITARGETSR ... GICD_ITARGETSRN: if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width; @@ -174,23 +148,10 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info) *r = 0xdeadbeef; return 1; + /* Setting/Clearing the SGI pending bit via GICD is not supported */ case GICD_CPENDSGIR ... GICD_CPENDSGIRN: - if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 1, gicd_reg - GICD_CPENDSGIR, DABT_WORD); - if ( rank == NULL) goto read_as_zero; - vgic_lock_rank(v, rank, flags); - *r = vgic_byte_read(rank->pendsgi, dabt.sign, gicd_reg); - vgic_unlock_rank(v, rank, flags); - return 1; - case GICD_SPENDSGIR ... GICD_SPENDSGIRN: - if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 1, gicd_reg - GICD_SPENDSGIR, DABT_WORD); - if ( rank == NULL) goto read_as_zero; - vgic_lock_rank(v, rank, flags); - *r = vgic_byte_read(rank->pendsgi, dabt.sign, gicd_reg); - vgic_unlock_rank(v, rank, flags); - return 1; + goto read_as_zero; /* Implementation defined -- read as zero */ case 0xfd0 ... 0xfe4: @@ -346,21 +307,17 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info) case GICD_ISACTIVER ... GICD_ISACTIVERN: if ( dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ISACTIVER, DABT_WORD); - if ( rank == NULL) goto write_ignore; - vgic_lock_rank(v, rank, flags); - rank->iactive &= ~*r; - vgic_unlock_rank(v, rank, flags); - return 1; + printk(XENLOG_G_ERR + "%pv: vGICD: unhandled word write %#"PRIregister" to ISACTIVER%d\n", + v, *r, gicd_reg - GICD_ISACTIVER); + return 0; case GICD_ICACTIVER ... GICD_ICACTIVERN: if ( dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 1, gicd_reg - GICD_ICACTIVER, DABT_WORD); - if ( rank == NULL) goto write_ignore; - vgic_lock_rank(v, rank, flags); - rank->iactive &= ~*r; - vgic_unlock_rank(v, rank, flags); - return 1; + printk(XENLOG_ERR + "%pv: vGICD: unhandled word write %#"PRIregister" to ICACTIVER%d\n", + v, *r, gicd_reg - GICD_ICACTIVER); + return 0; case GICD_ITARGETSR ... GICD_ITARGETSR + 7: /* SGI/PPI target is read only */ diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c index 609e3c8..c72eb6c 100644 --- a/xen/arch/arm/vgic-v3.c +++ b/xen/arch/arm/vgic-v3.c @@ -306,38 +306,16 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v, *r = rank->ienable; vgic_unlock_rank(v, rank, flags); return 1; + /* Read the pending status of an IRQ via GICD/GICR is not supported */ case GICD_ISPENDR ... GICD_ISPENDRN: - if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 1, reg - GICD_ISPENDR, DABT_WORD); - if ( rank == NULL ) goto read_as_zero; - vgic_lock_rank(v, rank, flags); - *r = vgic_byte_read(rank->ipend, dabt.sign, reg); - vgic_unlock_rank(v, rank, flags); - return 1; case GICD_ICPENDR ... GICD_ICPENDRN: - if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 1, reg - GICD_ICPENDR, DABT_WORD); - if ( rank == NULL ) goto read_as_zero; - vgic_lock_rank(v, rank, flags); - *r = vgic_byte_read(rank->ipend, dabt.sign, reg); - vgic_unlock_rank(v, rank, flags); - return 1; + goto read_as_zero; + + /* Read the active status of an IRQ via GICD/GICR is not supported */ case GICD_ISACTIVER ... GICD_ISACTIVERN: - if ( dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 1, reg - GICD_ISACTIVER, DABT_WORD); - if ( rank == NULL ) goto read_as_zero; - vgic_lock_rank(v, rank, flags); - *r = rank->iactive; - vgic_unlock_rank(v, rank, flags); - return 1; case GICD_ICACTIVER ... GICD_ICACTIVERN: - if ( dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 1, reg - GICD_ICACTIVER, DABT_WORD); - if ( rank == NULL ) goto read_as_zero; - vgic_lock_rank(v, rank, flags); - *r = rank->iactive; - vgic_unlock_rank(v, rank, flags); - return 1; + goto read_as_zero; + case GICD_IPRIORITYR ... GICD_IPRIORITYRN: if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width; rank = vgic_rank_offset(v, 8, reg - GICD_IPRIORITYR, DABT_WORD); @@ -415,36 +393,32 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v, return 1; case GICD_ISPENDR ... GICD_ISPENDRN: if ( dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 1, reg - GICD_ISPENDR, DABT_WORD); - if ( rank == NULL ) goto write_ignore; - vgic_lock_rank(v, rank, flags); - rank->ipend = *r; - vgic_unlock_rank(v, rank, flags); - return 1; + printk(XENLOG_G_ERR + "%pv: %s: unhandled word write %#"PRIregister" to ISPENDR%d\n", + v, name, *r, reg - GICD_ISPENDR); + return 0; + case GICD_ICPENDR ... GICD_ICPENDRN: if ( dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 1, reg - GICD_ICPENDR, DABT_WORD); - if ( rank == NULL ) goto write_ignore; - vgic_lock_rank(v, rank, flags); - rank->ipend &= ~*r; - vgic_unlock_rank(v, rank, flags); - return 1; + printk(XENLOG_G_ERR + "%pv: %s: unhandled word write %#"PRIregister" to ICPENDR%d\n", + v, name, *r, reg - GICD_ICPENDR); + return 0; + case GICD_ISACTIVER ... GICD_ISACTIVERN: if ( dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 1, reg - GICD_ISACTIVER, DABT_WORD); - if ( rank == NULL ) goto write_ignore; - vgic_lock_rank(v, rank, flags); - rank->iactive &= ~*r; - vgic_unlock_rank(v, rank, flags); - return 1; + printk(XENLOG_G_ERR + "%pv: %s: unhandled word write %#"PRIregister" to ISACTIVER%d\n", + v, name, *r, reg - GICD_ISACTIVER); + return 0; + case GICD_ICACTIVER ... GICD_ICACTIVERN: if ( dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 1, reg - GICD_ICACTIVER, DABT_WORD); - if ( rank == NULL ) goto write_ignore; - vgic_lock_rank(v, rank, flags); - rank->iactive &= ~*r; - vgic_unlock_rank(v, rank, flags); - return 1; + printk(XENLOG_G_ERR + "%pv: %s: unhandled word write %#"PRIregister" to ICACTIVER%d\n", + v, name, *r, reg - GICD_ICACTIVER); + return 0; + case GICD_IPRIORITYR ... GICD_IPRIORITYRN: if ( dabt.size != DABT_BYTE && dabt.size != DABT_WORD ) goto bad_width; rank = vgic_rank_offset(v, 8, reg - GICD_IPRIORITYR, DABT_WORD); @@ -496,8 +470,6 @@ static int vgic_v3_rdistr_sgi_mmio_read(struct vcpu *v, mmio_info_t *info, struct hsr_dabt dabt = info->dabt; struct cpu_user_regs *regs = guest_cpu_user_regs(); register_t *r = select_user_reg(regs, dabt.reg); - struct vgic_irq_rank *rank; - unsigned long flags; switch ( gicr_reg ) { @@ -517,22 +489,12 @@ static int vgic_v3_rdistr_sgi_mmio_read(struct vcpu *v, mmio_info_t *info, */ return __vgic_v3_distr_common_mmio_read("vGICR: SGI", v, info, gicr_reg); + + /* Read the pending status of an SGI is via GICR is not supported */ case GICR_ISPENDR0: - if ( dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 1, gicr_reg - GICR_ISPENDR0, DABT_WORD); - if ( rank == NULL ) goto read_as_zero; - vgic_lock_rank(v, rank, flags); - *r = rank->pendsgi; - vgic_unlock_rank(v, rank, flags); - return 1; case GICR_ICPENDR0: - if ( dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 1, gicr_reg - GICR_ICPENDR0, DABT_WORD); - if ( rank == NULL ) goto read_as_zero; - vgic_lock_rank(v, rank, flags); - *r = rank->pendsgi; - vgic_unlock_rank(v, rank, flags); - return 1; + goto read_as_zero; + case GICR_NSACR: /* We do not implement security extensions for guests, read zero */ goto read_as_zero_32; @@ -562,8 +524,6 @@ static int vgic_v3_rdistr_sgi_mmio_write(struct vcpu *v, mmio_info_t *info, struct hsr_dabt dabt = info->dabt; struct cpu_user_regs *regs = guest_cpu_user_regs(); register_t *r = select_user_reg(regs, dabt.reg); - struct vgic_irq_rank *rank; - unsigned long flags; switch ( gicr_reg ) { @@ -585,22 +545,18 @@ static int vgic_v3_rdistr_sgi_mmio_write(struct vcpu *v, mmio_info_t *info, info, gicr_reg); case GICR_ISPENDR0: if ( dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 1, gicr_reg - GICR_ISACTIVER0, DABT_WORD); - if ( rank == NULL ) goto write_ignore; - vgic_lock_rank(v, rank, flags); - /* TODO: we just store the SGI pending status. Handle it properly */ - rank->pendsgi |= *r; - vgic_unlock_rank(v, rank, flags); - return 1; + printk(XENLOG_G_ERR + "%pv: vGICR: SGI: unhandled word write %#"PRIregister" to ISPENDR0\n", + v, *r); + return 0; + case GICR_ICPENDR0: if ( dabt.size != DABT_WORD ) goto bad_width; - rank = vgic_rank_offset(v, 1, gicr_reg - GICR_ISACTIVER0, DABT_WORD); - if ( rank == NULL ) goto write_ignore; - vgic_lock_rank(v, rank, flags); - /* TODO: we just store the SGI pending status. Handle it properly */ - rank->pendsgi &= ~*r; - vgic_unlock_rank(v, rank, flags); - return 1; + printk(XENLOG_G_ERR + "%pv: vGICR: SGI: unhandled word write %#"PRIregister" to ICPENDR0\n", + v, *r); + return 0; + case GICR_NSACR: /* We do not implement security extensions for guests, write ignore */ goto write_ignore_32; @@ -620,7 +576,6 @@ bad_width: write_ignore_32: if ( dabt.size != DABT_WORD ) goto bad_width; -write_ignore: return 1; } diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h index 74d5a4e..0c7da7f 100644 --- a/xen/include/asm-arm/vgic.h +++ b/xen/include/asm-arm/vgic.h @@ -85,7 +85,7 @@ struct pending_irq /* Represents state corresponding to a block of 32 interrupts */ struct vgic_irq_rank { spinlock_t lock; /* Covers access to all other members of this struct */ - uint32_t ienable, iactive, ipend, pendsgi; + uint32_t ienable; uint32_t icfg[2]; uint32_t ipriority[8]; union {