From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: RE: use of struct hvm_mirq_dpci_mapping.gmsi vs. HVM_IRQ_DPCI_*_MSI flags Date: Wed, 27 Apr 2011 07:43:22 +0100 Message-ID: <4DB7D72A020000780003E4C4@vpn.id2.novell.com> References: <4D94A88C0200007800039637@vpn.id2.novell.com> <4DB6A2EE020000780003E1DC@vpn.id2.novell.com> <987664A83D2D224EAE907B061CE93D5301C50FD4BF@orsmsx505.amr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <987664A83D2D224EAE907B061CE93D5301C50FD4BF@orsmsx505.amr.corp.intel.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Allen M Kay Cc: Haitao Shan , "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org >>> On 27.04.11 at 04:49, "Kay, Allen M" wrote: >> > >>> I'm largely asking because I think struct hvm_mirq_dpci_mapping.dom >>> and .digl_list could actually overlay .gmsi, as much as struct >>> hvm_irq_dpci.hvm_timer could actually rather be folded into struct >>> hvm_mirq_dpci_mapping (and then also overlay .gmsi). The overlay >>> distinction bit would, based on initialization, be HVM_IRQ_DPCI_GUEST_M= SI, >>> but according to use it wouldn't be clear which of the two >>> HVM_IRQ_DPCI_*_MSI bits is actually the correct one. >>> >=20 > Jan, sorry for the late reply. I was out of the office in the past = week. >=20 > Are you proposing the following data structure change? >=20 > struct hvm_mirq_dpci_mapping { > uint32_t flags; > int pending; > union { > struct timer *hvm_timer; > struct list_head_digl_list; > struct domain *dom; > struct hvm_gmsi_info gmsi; > }; > } No - afaics timer, digl_list, and dom must be usable at the same time, so only gmsi is an actual overlay (union) candidate. But then again there's not that much of a significance to this anymore once these won't get allocated as arrays, so it's more of a second level optimization. Also, with my current (not yet posted) implementation there won't be arrays of pointers either, instead there'll be a radix tree (indexed by guest pirq) with pointers attached. So it'll be a per-domain structure struct hvm_irq_dpci { /* Guest IRQ to guest device/intx mapping. */ struct list_head girq[NR_HVM_IRQS]; /* Record of mapped ISA IRQs */ DECLARE_BITMAP(isairq_map, NR_ISAIRQS); /* Record of mapped Links */ uint8_t link_cnt[NR_LINK]; struct tasklet dirq_tasklet; }; and a per-guest-pirq one struct hvm_pirq_dpci { uint32_t flags; bool_t masked; uint16_t pending; struct list_head digl_list; struct domain *dom; struct hvm_gmsi_info gmsi; struct timer timer; }; which possibly in a second step could become struct hvm_pirq_dpci { uint32_t flags; bool_t masked; uint16_t pending; union { struct { struct list_head digl_list; struct domain *dom; struct timer timer; } pci; struct { uint32_t gvec; uint32_t gflags; int dest_vcpu_id; /* -1 :multi-dest, non-negative: dest_vcpu_id= */ } msi; }; }; But clarification on the current (perhaps vs intended) use of HVM_IRQ_DPCI_*_MSI would still be much appreciated (and if, as suspected, there's need to clean this up, I'd like the cleanup to be done before the patches I have pending). Also, there is one more open question (quoting the mail titled "pt_irq_time_out() dropping d->event_lock before calling pirq_guest_eoi()"): "What is the reason for this? irq_desc's lock nests inside d->event_lock, and not having to drop the lock early would not only allow the two loops to be folded, but also to call a short cut version of pirq_guest_eoi() that already obtained the pirq->irq mapping (likely going to be created when splitting the d->nr_pirqs sized arrays I'm working on currently)." In my pending patches I imply that this separation is indeed unnecessary. Jan