Re: Why does xc_map_foreign_range() refuse to map pfns below 1M from a domU

[Tomasz Wroblewski <tomasz.wroblewski@citrix.com>]

[-- Attachment #1: Type: text/plain, Size: 3264 bytes --]

On 12/03/2013 08:07 PM, Konrad Rzeszutek Wilk wrote:
> On Tue, Dec 03, 2013 at 06:36:48PM +0100, Tomasz Wroblewski wrote:
>> On 12/03/2013 05:09 PM, Ian Campbell wrote:
>>> On Tue, 2013-12-03 at 17:59 +0200, Razvan Cojocaru wrote:
>>>>>> The Linux domU is perfectly able to map (using xc_map_foreign_range())
>>>>>> pages from the Windows domU, except for pages below 1M.
>>>>>
>>>>> With no XSM how does it have the privilege to do this?
>>>>
>>>> What I meant to say is that the domU is being allowed to do this sort
>>>> of thing, i.e. the problem is definitely not caused by XSM.
>>>
>>> OK, so XSM is involved but you are 101% certain that it is not
>>> preventing the mappings?
>>>
>> We've ran into this issue in xenclient recently too, when we finally
>> upgraded stubdomain's kernel to pvops version. It seems pvops kernel
>> contains safeguard to only allow <1M mappings if it's dom0
>> (xen_initial_domain()). This check is placed in arch/x86/xen/mmu.c:
>>
>> static pte_t xen_make_pte(pteval_t pte)
>> {
>>          phys_addr_t addr = (pte & PTE_PFN_MASK);
>>
>> ...
>>          /*
>>           * Unprivileged domains are allowed to do IOMAPpings for
>>           * PCI passthrough, but not map ISA space.  The ISA
>>           * mappings are just dummy local mappings to keep other
>>           * parts of the kernel happy.
>>           */
>>          if (unlikely(pte & _PAGE_IOMAP) &&
>>              (xen_initial_domain() || addr >= ISA_END_ADDRESS)) {
>>                  pte = iomap_pte(pte);
>>          } else {
>>                  pte &= ~_PAGE_IOMAP;
>>                  pte = pte_pfn_to_mfn(pte);
>>          }
>>
>>          return native_make_pte(pte);
>> }
>>
>> We patched this out (in a fugly and probably not very correct way),
>> for our stubdomain kernel, since we needed our stubdomain qemu vms
>> to be able to map windows guest <1M range (since qemu needs to be
>> able to write data and read data there in order to chat with seabios
>> etc). Maybe Konrad (CC'ed) knows why the check is there in guest
>> kernel, and a good way to solve this.
>
> For PV domU guests the ISA are usually RAM - so you don't want during
> early bootup of a PV guest for it to scan MFNs it does not have access
> to. Granted it does not have access to them but it would have the
> MFNs coded in and any access to that area will result in .. Xen
> "fixing" up the PTEs (I can't recall exaclty how).
>
> If you boot a PV Guest and remove the:
>               (xen_initial_domain() || addr >= ISA_END_ADDRESS)) {
>
> do you see anything that in the Xen console?
>
I recall I wasn't seeing anything, the pv domU was just hanging super early in the boot then. The way we worked around it is via attached 
patch (applied to PV domU's kernel, in our case stubdom hosting qemu process). It keeps the <1M safeguard for local mapping but allows 
foreign mappings (detected via _PAGE_SPECIAL flag).

Razvan, you can try attached patch as well applied to your pv domU kernel to see if it helps you.




>>
>> I think the goal of check was to only stop <1M mapping of its own
>> memory in order to stop pvops kernel boot messing it, but by
>> ricochet it also prevents mapping of foreign domain <1M ranges...
>
> Duh! That was certainly unintentional.
>
>>
>>
>>


[-- Attachment #2: stubdom-allow-foreign-lowmem-map --]
[-- Type: text/plain, Size: 1317 bytes --]

diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index cab96b6..dafd70d 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -502,8 +502,11 @@ static pte_t xen_make_pte(pteval_t pte)
 	 * mappings are just dummy local mappings to keep other
 	 * parts of the kernel happy.
 	 */
+
+        /* stubdom: we allow the mapping of lowmem of another domain, marked via
+         * simultaneous _PAGE_SPECIAL and _PAGE_IOMAP bits */
 	if (unlikely(pte & _PAGE_IOMAP) &&
-	    (xen_initial_domain() || addr >= ISA_END_ADDRESS)) {
+	    (xen_initial_domain() || addr >= ISA_END_ADDRESS) || (pte & _PAGE_SPECIAL)) {
 		pte = iomap_pte(pte);
 	} else {
 		pte &= ~_PAGE_IOMAP;
@@ -2483,11 +2486,18 @@ struct remap_data {
 	struct mmu_update *mmu_update;
 };
 
+static inline pte_t foreign_special_pfn_pte(unsigned long page_nr, pgprot_t pgprot)
+{
+	return __pte(((phys_addr_t)page_nr << PAGE_SHIFT) |
+		     massage_pgprot(pgprot) | _PAGE_SPECIAL);
+}
+
+
 static int remap_area_mfn_pte_fn(pte_t *ptep, pgtable_t token,
 				 unsigned long addr, void *data)
 {
 	struct remap_data *rmd = data;
-	pte_t pte = pte_mkspecial(pfn_pte(rmd->mfn++, rmd->prot));
+	pte_t pte = foreign_special_pfn_pte(rmd->mfn++, rmd->prot);
 
 	rmd->mmu_update->ptr = virt_to_machine(ptep).maddr;
 	rmd->mmu_update->val = pte_val_ma(pte);

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel