Re: [Xen-devel] [v7][PATCH 06/16] hvmloader/pci: skip reserved ranges

2015-07-15 Thread Chen, Tiejun
On 2015/7/15 19:27, Jan Beulich wrote: On 15.07.15 at 13:05, george.dun...@eu.citrix.com wrote: This patch series as a whole represents a lot of work and a lot of tangible improvements to the situation; and (unless the situation has changed) it's almost entirely acked apart from the MMIO

Re: [Xen-devel] [v7][PATCH 06/16] hvmloader/pci: skip reserved ranges

2015-07-15 Thread Chen, Tiejun
Yet more special casing code you want to add. I said no to this model, and unless you can address the issue _without_ adding a lot of special casing code, the answer will remain no (subject What about this? @@ -301,6 +301,19 @@ void pci_setup(void) pci_mem_start = 1; } +

Re: [Xen-devel] [v7][PATCH 06/16] hvmloader/pci: skip reserved ranges

2015-07-15 Thread Chen, Tiejun
Is not booting worse than what we have now -- which is, booting successfully but (probably) having issues due to MMIO ranges overlapping RMRRs? Its really so rare possibility here since in the real world we didn't see any RMRR regions = 0xF000 (the default pci memory start.) And I already

Re: [Xen-devel] [v7][PATCH 13/16] libxl: construct e820 map with RDM information for HVM guest

2015-07-14 Thread Chen, Tiejun
However long term I think it might make sense to try and reuse one of the existing libxl__arch hooks, i.e. libxl__arch_domain_init_hw_description or libxl__arch_domain_finalise_hw_description. On ARM these are to do with setting the Device Tree Blob, which included the memory map, so it is

Re: [Xen-devel] Requesting for freeze exception for RMRR

2015-07-14 Thread Chen, Tiejun
Y Y [v7][PATCH 14/16] xen/vtd: enable USB device assignment Y Y [v7][PATCH 15/16] xen/vtd: prevent from assign the device with shared rmrr And yet again for these two. Please avoid giving a false impression But these two patches really won Kevin's Ack,

Re: [Xen-devel] [v7][PATCH 06/16] hvmloader/pci: skip reserved ranges

2015-07-14 Thread Chen, Tiejun
Note here I don't address your comments above since I think we should achieve an agreement firstly. I think bitmap mechanism is a good idea but honestly, its not easy to cover all requirements here. And just like bootmem on Linux side, so its a little complicated to implement this entirely.

Re: [Xen-devel] [v7][PATCH 07/16] hvmloader/e820: construct guest e820 table

2015-07-14 Thread Chen, Tiejun
On 2015/7/14 17:32, Jan Beulich wrote: On 14.07.15 at 07:22, tiejun.c...@intel.com wrote: +for ( i = 0; i memory_map.nr_map; i++ ) +{ +uint64_t end = e820[i].addr + e820[i].size; Either loop index/boundary or used array are wrong here: In the earlier loop you copied

Re: [Xen-devel] [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-14 Thread Chen, Tiejun
The way this sort of thing is defined in the rest of domctl.h is like this: #define _XEN_DOMCTL_CDF_hvm_guest 0 #define XEN_DOMCTL_CDF_hvm_guest (1U_XEN_DOMCTL_CDF_hvm_guest) So the above should be #define _XEN_DOMCTL_DEV_RDM_RELAXED 0 #define XEN_DOMCTL_DEV_RDM_RELAXED

Re: [Xen-devel] [v7][PATCH 06/16] hvmloader/pci: skip reserved ranges

2015-07-14 Thread Chen, Tiejun
-} *resource, mem_resource, high_mem_resource, io_resource; +} *resource, mem_resource, high_mem_resource, io_resource, exp_mem_resource; Despite having gone through description and the rest of the patch I can't seem to be able to guess what exp_mem stands for. Meaningful variable

Re: [Xen-devel] [v7][PATCH 06/16] hvmloader/pci: skip reserved ranges

2015-07-14 Thread Chen, Tiejun
I agree we'd better overhaul this since we already found something unreasonable here. But one or two weeks is really not enough to fix this with a bitmap framework, and although a bitmap can make mmio allocation better, but more complicated if we just want to allocate PCI mmio. So could we do

Re: [Xen-devel] [v7][PATCH 06/16] hvmloader/pci: skip reserved ranges

2015-07-14 Thread Chen, Tiejun
On 2015/7/15 8:55, Chen, Tiejun wrote: I agree we'd better overhaul this since we already found something unreasonable here. But one or two weeks is really not enough to fix this with a bitmap framework, and although a bitmap can make mmio allocation better, but more complicated if we just want

Re: [Xen-devel] [v7][PATCH 16/16] tools: parse to enable new rdm policy parameters

2015-07-13 Thread Chen, Tiejun
On 2015/7/13 17:40, Ian Campbell wrote: On Mon, 2015-07-13 at 17:31 +0800, Chen, Tiejun wrote: I still can't understand what I'm missing here after compared to other contexts inside xlu_pci_parse_bdf(). Perhaps comparing to the CODING_STYLE document would help? Looks the whole

Re: [Xen-devel] [v7][PATCH 11/16] tools/libxl: detect and avoid conflicts with RDM

2015-07-13 Thread Chen, Tiejun
Do you mean I should merge them as one as possible? Factor it out means to break out into a separate function (or maybe a macro or something, but in this case a function is appropriate). So in this case take the two sets of similar code, combine them into a function with appropriate arguments,

Re: [Xen-devel] [v7][PATCH 16/16] tools: parse to enable new rdm policy parameters

2015-07-13 Thread Chen, Tiejun
+}else if ( !strcmp(optkey, rdm_policy) ) { +if ( !strcmp(tok, strict) ) { +pcidev-rdm_policy = LIBXL_RDM_RESERVE_POLICY_STRICT; +} else if ( !strcmp(tok, relaxed) ) { +pcidev-rdm_policy =

Re: [Xen-devel] [v7][PATCH 13/16] libxl: construct e820 map with RDM information for HVM guest

2015-07-13 Thread Chen, Tiejun
This approach looks like it should work, and I think given the point in the release it would be acceptable for 4.6. However long term I think it might make sense to try and reuse one of the existing libxl__arch hooks, i.e. libxl__arch_domain_init_hw_description or

Re: [Xen-devel] [v7][PATCH 13/16] libxl: construct e820 map with RDM information for HVM guest

2015-07-13 Thread Chen, Tiejun
On 2015/7/13 18:15, Ian Campbell wrote: On Mon, 2015-07-13 at 17:47 +0800, Chen, Tiejun wrote: This approach looks like it should work, and I think given the point in the release it would be acceptable for 4.6. However long term I think it might make sense to try and reuse one of the existing

Re: [Xen-devel] [v7][PATCH 07/16] hvmloader/e820: construct guest e820 table

2015-07-13 Thread Chen, Tiejun
Now we can use that memory map to build our final e820 table but it may need to reorder all e820 entries. it being what? I'm afraid I can't really make sense of the second half of the sentence... I hope the following can work for you, ... but finally we should sort them into an increasing

Re: [Xen-devel] Requesting for freeze exception for RMRR

2015-07-13 Thread Chen, Tiejun
1. clarify the state of patch series / feature. ReviewedAcked RMRR series v7 Y Y [v7][PATCH 01/16] xen: introduce XENMEM_reserved_device_memory_map Y Y [v7][PATCH 02/16] xen/vtd: create RMRR mapping Y N [v7][PATCH 03/16]

Re: [Xen-devel] Requesting for freeze exception for RMRR

2015-07-13 Thread Chen, Tiejun
Please work with maintainers to get those hvmloader patches acked or reviewed. I will do. Note Jackson and Campbell also raised some comments to improve current codes. 2. explain why it needs to be in this release (benefits). RMRR mechanism was broken for a long time and this makes VM

Re: [Xen-devel] [v7][PATCH 16/16] tools: parse to enable new rdm policy parameters

2015-07-13 Thread Chen, Tiejun
On 2015/7/14 1:08, Ian Jackson wrote: Ian Campbell writes (Re: [Xen-devel] [v7][PATCH 16/16] tools: parse to enable new rdm policy parameters): On Mon, 2015-07-13 at 17:55 +0800, Chen, Tiejun wrote: So I can do this as you're expecting now, but seems our change would make the code style very

Re: [Xen-devel] [v7][PATCH 15/16] xen/vtd: prevent from assign the device with shared rmrr

2015-07-13 Thread Chen, Tiejun
+{ +printk(XENLOG_G_ERR VTDPREFIX +cannot assign %04x:%02x:%02x.%u +with shared RMRR for Dom%d.\n, + seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), + d-domain_id); +

Re: [Xen-devel] [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-13 Thread Chen, Tiejun
Thanks for this; a few more comments... Thanks for your time. @@ -1577,9 +1578,15 @@ int iommu_do_pci_domctl( seg = machine_sbdf 16; bus = PCI_BUS(machine_sbdf); devfn = PCI_DEVFN2(machine_sbdf); +flag = domctl-u.assign_device.flag; +if ( flag

[Xen-devel] Requesting for freeze exception for RMRR

2015-07-13 Thread Chen, Tiejun
Hi Wei, Here I'm trying to request the freeze exception for RMRR. 1. clarify the state of patch series / feature. ReviewedAcked RMRR series v7 Y Y [v7][PATCH 01/16] xen: introduce XENMEM_reserved_device_memory_map Y Y [v7][PATCH 02/16]

Re: [Xen-devel] [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-13 Thread Chen, Tiejun
I was saying two things in the above paragraph: 1. For removal, there's no point in passing in anything other than '0' for flags, since it's ignored. Passing a non-0 value implies that the flags will have some effect, which is misleading. 2. For places we know we're adding to hw domains, I

Re: [Xen-devel] [v7][PATCH 05/16] hvmloader: get guest memory map into memory_map[]

2015-07-13 Thread Chen, Tiejun
On 2015/7/10 21:49, George Dunlap wrote: On Thu, Jul 9, 2015 at 6:33 AM, Tiejun Chen tiejun.c...@intel.com wrote: Now we get this map layout by call XENMEM_memory_map then save them into one global variable memory_map[]. It should include lowmem range, rdm range and highmem range. Note rdm

Re: [Xen-devel] [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-12 Thread Chen, Tiejun
Also, passing in RELAXED in locations where the flag is completely ignored (such as when removing mappings) doesn't really make any sense. On the whole I think it would be better if you removed the RELAXED flag for both removals and for hardware domains. But what would he pass instead? Or wait

Re: [Xen-devel] [v7][PATCH 16/16] tools: parse to enable new rdm policy parameters

2015-07-10 Thread Chen, Tiejun
The first issue (which would really be relevant to the documentation patch) is that the documentation is in a separate commit. There are sometimes valid reasons for doing this. I'm not sure if they apply, Wei suggested we should organize/spit all patches according to libxl, libxc, xc and xl.

Re: [Xen-devel] [v7][PATCH 13/16] libxl: construct e820 map with RDM information for HVM guest

2015-07-09 Thread Chen, Tiejun
tools/libxl/libxl_dom.c | 5 +++ tools/libxl/libxl_internal.h | 24 + tools/libxl/libxl_x86.c | 83 ... diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index 62ef120..41da479 100644 ---

Re: [Xen-devel] [v7][PATCH 10/16] tools: introduce some new parameters to set rdm policy

2015-07-09 Thread Chen, Tiejun
int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci) { +/* We'd like to force reserve rdm specific to a device by default.*/ +if ( pci-rdm_policy == LIBXL_RDM_RESERVE_POLICY_INVALID) ^ I have just spotted that spurious whitespace. However I won't block

Re: [Xen-devel] [v7][PATCH 11/16] tools/libxl: detect and avoid conflicts with RDM

2015-07-09 Thread Chen, Tiejun
I have found a few things in this patch which I would like to see improved. See below. Given how late I am with this review, I do not feel that I should be nacking it at this time. You have a tools ack from Wei, so my comments are not a blocker for this series. But if you need to respin,

Re: [Xen-devel] [v7][PATCH 10/16] tools: introduce some new parameters to set rdm policy

2015-07-09 Thread Chen, Tiejun
On 2015/7/9 18:37, Ian Jackson wrote: Chen, Tiejun writes (Re: [v7][PATCH 10/16] tools: introduce some new parameters to set rdm policy): Note I also mask patch #10 Acked by Wei Liu, Ian Jackson and Ian Campbell. ( If I'm wrong just let me know at this point. )... For future reference, if I

Re: [Xen-devel] [v7][PATCH 11/16] tools/libxl: detect and avoid conflicts with RDM

2015-07-09 Thread Chen, Tiejun
CC: Ian Jackson ian.jack...@eu.citrix.com CC: Stefano Stabellini stefano.stabell...@eu.citrix.com CC: Ian Campbell ian.campb...@citrix.com CC: Wei Liu wei.l...@citrix.com Acked-by: Wei Liu wei.l...@citrix.com Signed-off-by: Tiejun Chen tiejun.c...@intel.com Reviewed-by: Kevin Tian

Re: [Xen-devel] [v7][PATCH 10/16] tools: introduce some new parameters to set rdm policy

2015-07-09 Thread Chen, Tiejun
Acked-by: Wei Liu wei.l...@citrix.com Acked-by: Ian Jackson ian.jack...@eu.citrix.com Acked-by: Ian Campbell ian.campb...@citrix.com Signed-off-by: Tiejun Chen tiejun.c...@intel.com FWIW you shouldn't put Ian and Ian's acks here because they have not explicitly done so. Yes, I'm not very sure

Re: [Xen-devel] [v5][PATCH 10/16] tools: introduce some new parameters to set rdm policy

2015-07-08 Thread Chen, Tiejun
I'll update this next revision. And also rephrase this doc to address your comments below. FTR I think I indicated yesterday that I was satisfied with your explanation for why type=none exists as an option even at the xl level, namely that it allows us to change the default in the future.

Re: [Xen-devel] [v6][PATCH 10/16] tools: introduce some new parameters to set rdm policy

2015-07-08 Thread Chen, Tiejun
On 2015/7/8 21:27, Ian Jackson wrote: Chen, Tiejun writes (Re: [v6][PATCH 10/16] tools: introduce some new parameters to set rdm policy): On 2015/7/8 19:47, Ian Jackson wrote: I appreciate that I have come to this review late. While I have found the review conversation quite unsatisfactory

Re: [Xen-devel] [v5][PATCH 10/16] tools: introduce some new parameters to set rdm policy

2015-07-07 Thread Chen, Tiejun
I mean if you don't set these mappings, these devices can't work at all and then crash VM like IGD passthrough. But I'm also saying we don't pass through any devices in most cases, and those devices which own RDM are very rare. So why should we set 'none' to Xen by default? One typo,

Re: [Xen-devel] [v4][PATCH 04/19] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-07 Thread Chen, Tiejun
Just please go to review the new revision directly. Thanks Tiejun On 2015/7/6 22:52, Chen, Tiejun wrote: Any comment to this? Thanks Tiejun On 2015/7/2 16:49, Chen, Tiejun wrote: @@ -1898,7 +1899,13 @@ static int intel_iommu_add_device(u8 devfn, struct pci_dev *pdev) PCI_BUS

Re: [Xen-devel] [v4][PATCH 04/19] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-07 Thread Chen, Tiejun
Just please go to review the new revision directly. Thanks Tiejun On 2015/7/6 22:55, Chen, Tiejun wrote: * for dt devices: - Ignore this flag entirely But we still a flag to assign_device() like this, diff --git a/xen/drivers/passthrough/device_tree.c b/xen/drivers/passthrough

Re: [Xen-devel] [v5][PATCH 10/16] tools: introduce some new parameters to set rdm policy

2015-07-07 Thread Chen, Tiejun
On 2015/7/8 1:08, Ian Jackson wrote: Chen, Tiejun writes (Re: [v5][PATCH 10/16] tools: introduce some new parameters to set rdm policy): Its always fine to me but I just think, is it a good time to start to seek another *optional* approach to overturn current design and implementation ? Unless

Re: [Xen-devel] [v5][PATCH 01/16] xen: introduce XENMEM_reserved_device_memory_map

2015-07-07 Thread Chen, Tiejun
+/* IN/OUT */ +unsigned intnr_entries; Perhaps I am missing something but I can't find any API documentation for the return value and error returns from this new hypercall. I think this is in line with everything else in this header - am I overlooking something? In particular,

Re: [Xen-devel] [v5][PATCH 10/16] tools: introduce some new parameters to set rdm policy

2015-07-07 Thread Chen, Tiejun
On 2015/7/7 22:40, Ian Jackson wrote: Chen, Tiejun writes (Re: [v5][PATCH 10/16] tools: introduce some new parameters to set rdm policy): On 2015/7/7 21:26, Ian Jackson wrote: Is none not hoping the user can ignore the problem ? Its impossible since the hypervisor or tools can't prevent

Re: [Xen-devel] [v5][PATCH 11/16] tools/libxl: detect and avoid conflicts with RDM

2015-07-07 Thread Chen, Tiejun
On 2015/7/7 22:57, Ian Jackson wrote: Chen, Tiejun writes (Re: [v5][PATCH 11/16] tools/libxl: detect and avoid conflicts with RDM): On 2015/7/7 19:57, Ian Jackson wrote: I think the error handling here is wrong. Certainly `rc' should not be used for a libxc return. Nope, we don't return rc

Re: [Xen-devel] [v5][PATCH 10/16] tools: introduce some new parameters to set rdm policy

2015-07-07 Thread Chen, Tiejun
Global RDM parameter, type, allows user to specify reserved regions explicitly, e.g. using 'host' to include all reserved regions reported As I understand this feature, I don't think the name `type' is right. This is a method for handling or overriding/ignoring the RDM problem. Perhaps

Re: [Xen-devel] [v5][PATCH 10/16] tools: introduce some new parameters to set rdm policy

2015-07-07 Thread Chen, Tiejun
+none is the default value and it means we don't check any reserved regions +and then all rdm policies would be ignored. Guest just works as before and +the conflict of RDM and guest address space wouldn't be handled, and then +this may result in the associated device not being able to work or

Re: [Xen-devel] [v5][PATCH 10/16] tools: introduce some new parameters to set rdm policy

2015-07-07 Thread Chen, Tiejun
On 2015/7/7 21:26, Ian Jackson wrote: Chen, Tiejun writes (Re: [v5][PATCH 10/16] tools: introduce some new parameters to set rdm policy): [Later:] As I discussed with Campbell we'd like not to expose none in xl level since this is equivalent to that case we don't set anything. I think

Re: [Xen-devel] [v5][PATCH 11/16] tools/libxl: detect and avoid conflicts with RDM

2015-07-07 Thread Chen, Tiejun
On 2015/7/7 19:57, Ian Jackson wrote: Tiejun Chen writes ([v5][PATCH 11/16] tools/libxl: detect and avoid conflicts with RDM): While building a VM, HVM domain builder provides struct hvm_info_table{} to help hvmloader. Currently it includes two fields to construct guest e820 table by

Re: [Xen-devel] [v4][PATCH 04/19] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-06 Thread Chen, Tiejun
Yes, this demonstrates my point. Each of these is a single-bit boolean value that takes up a single bit -- either on or off. But here you have three values -- NO_DRM, RELAXED, and STRICT, that take up two bits. If Is this fine to you? #define _XEN_DOMCTL_DEV_NO_RDM 0 #define

Re: [Xen-devel] [v4][PATCH 11/19] tools: introduce some new parameters to set rdm policy

2015-07-06 Thread Chen, Tiejun
I think we should go back here. I was involved in the design discussion, and from the very beginning I probably saw your plan but misunderstood it. I wouldn't be surprised if some others didn't quite understand what they were agreeing to. This way of doing things is different than the way we

Re: [Xen-devel] [v4][PATCH 04/19] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-06 Thread Chen, Tiejun
* for dt devices: - Ignore this flag entirely But we still a flag to assign_device() like this, diff --git a/xen/drivers/passthrough/device_tree.c b/xen/drivers/passthrough/device_tree.c index 5d3842a..a182487 100644 --- a/xen/drivers/passthrough/device_tree.c +++

Re: [Xen-devel] [v4][PATCH 11/19] tools: introduce some new parameters to set rdm policy

2015-07-06 Thread Chen, Tiejun
On 2015/7/6 22:34, Jan Beulich wrote: On 06.07.15 at 16:29, george.dun...@eu.citrix.com wrote: It sounds like part of the problem here is a matter of domains. Jan cares mostly about what happens in the hypervisor. At the hypervisor level, there is only the per-device configurations, and he is

Re: [Xen-devel] [v4][PATCH 04/19] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-06 Thread Chen, Tiejun
Any comment to this? Thanks Tiejun On 2015/7/2 16:49, Chen, Tiejun wrote: @@ -1898,7 +1899,13 @@ static int intel_iommu_add_device(u8 devfn, struct pci_dev *pdev) PCI_BUS(bdf) == pdev-bus PCI_DEVFN2(bdf) == devfn ) { -ret

Re: [Xen-devel] [v4][PATCH 11/19] tools: introduce some new parameters to set rdm policy

2015-07-06 Thread Chen, Tiejun
This way of doing things is different than the way we do it with most other options relating to pci devices (e.g., pci_permissive, pci_msitranslate, pci_sieze, c). All of those options use a default semantic: the domain-wide setting takes effect only if it's not set locally. If the syntax looks

Re: [Xen-devel] [v4][PATCH 04/19] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-02 Thread Chen, Tiejun
When you say not tools, I take it to mean that you're not exposing that option through the libxl interface? Yes. tools/libxc/xc_domain.c:xc_assign_dt_device() most certainly does pass it in, and that's the level I'm talking about. Someone reviewing this patch series needs to know, when xc

Re: [Xen-devel] [v4][PATCH 11/19] tools: introduce some new parameters to set rdm policy

2015-07-02 Thread Chen, Tiejun
On 2015/7/2 18:28, George Dunlap wrote: On 07/02/2015 11:01 AM, Chen, Tiejun wrote: 1. After spending yet another half hour doing research, I haven't found any discussion that concluded we should have the global policy override the local policy I also took some time to go back checking

Re: [Xen-devel] [v4][PATCH 04/19] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-02 Thread Chen, Tiejun
@@ -1898,7 +1899,13 @@ static int intel_iommu_add_device(u8 devfn, struct pci_dev *pdev) PCI_BUS(bdf) == pdev-bus PCI_DEVFN2(bdf) == devfn ) { -ret = rmrr_identity_mapping(pdev-domain, 1, rmrr); +/* + * RMRR is always

Re: [Xen-devel] [v4][PATCH 11/19] tools: introduce some new parameters to set rdm policy

2015-07-02 Thread Chen, Tiejun
in high level I have to say Yes. If you really read that v2 design and its associated discussion, you should notice I didn't put any response right there. Look, I'm getting a bit angry at your continual implication that I Sorry to this. haven't put in enough work reading the background for

Re: [Xen-devel] [v4][PATCH 04/19] xen/passthrough: extend hypercall to support rdm reservation policy

2015-07-01 Thread Chen, Tiejun
On 2015/7/1 18:02, George Dunlap wrote: On 07/01/2015 02:11 AM, Chen, Tiejun wrote: On 2015/6/30 22:20, George Dunlap wrote: On 06/30/2015 12:24 PM, Chen, Tiejun wrote: +#define XEN_DOMCTL_DEV_NO_RDM 0 +#define XEN_DOMCTL_DEV_RDM_RELAXED 1 +#define XEN_DOMCTL_DEV_RDM_STRICT

Re: [Xen-devel] [v4][PATCH 11/19] tools: introduce some new parameters to set rdm policy

2015-07-01 Thread Chen, Tiejun
1. By default, the domain policy is RELAXED (See above, libxl__rdm_setdefault()). 2. By default, the policy for individual devices is STRICT (see libxl_pci.c:libxl__device_pci_setdefault()) 3. If the domain policy is set to STRICT, this overrides per-device policy 4. If the domain policy is

Re: [Xen-devel] [v4][PATCH 11/19] tools: introduce some new parameters to set rdm policy

2015-07-01 Thread Chen, Tiejun
On 2015/7/1 18:57, George Dunlap wrote: On 07/01/2015 11:26 AM, Chen, Tiejun wrote: 1. By default, the domain policy is RELAXED (See above, libxl__rdm_setdefault()). 2. By default, the policy for individual devices is STRICT (see libxl_pci.c:libxl__device_pci_setdefault()) 3. If the domain

Re: [Xen-devel] [v4][PATCH 12/19] tools/libxl: passes rdm reservation policy

2015-07-01 Thread Chen, Tiejun
This is trying to covert LIBXL_XXX to XEN_XXX passed this policy as a hypercall, so I still think this is better to live here. Instead, the previous patch is just defining something. The entire rest of this patch is about xl. It doesn't make any sense at all for the previous patch to modify

Re: [Xen-devel] [v4][PATCH 11/19] tools: introduce some new parameters to set rdm policy

2015-07-01 Thread Chen, Tiejun
I don't happen to think these override semantics are actually going to turn out to be that useful; I do think a default semantic would be useful. But I'd be content if the name of the current setting were switched to override to make the semantics more clear. We can always add in default at

Re: [Xen-devel] Xen 4.6 Development Update (2 WEEKS TO FREEZE, important information in preamble)

2015-07-01 Thread Chen, Tiejun
* RMRR fix (fair) RFC posted Wei, I think this should be ok or good based on current status, and also should remove RFC here. Thanks Tiejun ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel

Re: [Xen-devel] [v4][PATCH 11/19] tools: introduce some new parameters to set rdm policy

2015-07-01 Thread Chen, Tiejun
If I'm correct, then #3 means it's not possible to have devices for a domain *default* to strict, but to be relaxed in individual instances. If you had five devices you wanted strict, and only one device you wanted to be relaxed (because you knew it didn't matter), you'd have to set

Re: [Xen-devel] [v4][PATCH 04/19] xen/passthrough: extend hypercall to support rdm reservation policy

2015-06-30 Thread Chen, Tiejun
On 2015/6/30 22:20, George Dunlap wrote: On 06/30/2015 12:24 PM, Chen, Tiejun wrote: +#define XEN_DOMCTL_DEV_NO_RDM 0 +#define XEN_DOMCTL_DEV_RDM_RELAXED 1 +#define XEN_DOMCTL_DEV_RDM_STRICT 2 +uint32_t flag; /* flag of assigned device */ Normally flags would

Re: [Xen-devel] [v4][PATCH 11/19] tools: introduce some new parameters to set rdm policy

2015-06-30 Thread Chen, Tiejun
On 2015/6/30 23:54, George Dunlap wrote: On Tue, Jun 23, 2015 at 10:57 AM, Tiejun Chen tiejun.c...@intel.com wrote: @@ -1450,6 +1458,11 @@ static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *multidev, } for (i = 0; i d_config-num_pcidevs; i++) { +/* +

Re: [Xen-devel] [v4][PATCH 12/19] tools/libxl: passes rdm reservation policy

2015-06-30 Thread Chen, Tiejun
@@ -988,6 +988,14 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i out: if (!libxl_is_stubdom(ctx, domid, NULL)) { +if (pcidev-rdm_reserve == LIBXL_RDM_RESERVE_FLAG_RELAXED) { +flag = XEN_DOMCTL_DEV_RDM_RELAXED; +} else if

Re: [Xen-devel] [v4][PATCH 12/19] tools/libxl: passes rdm reservation policy

2015-06-30 Thread Chen, Tiejun
On 2015/7/1 0:11, George Dunlap wrote: On Tue, Jun 23, 2015 at 10:57 AM, Tiejun Chen tiejun.c...@intel.com wrote: This patch passes our rdm reservation policy inside libxl when we assign a device or attach a device. Actually, it looks like what you need to do here, both for this patch and the

Re: [Xen-devel] [v4][PATCH 11/19] tools: introduce some new parameters to set rdm policy

2015-06-30 Thread Chen, Tiejun
The code fragment didn't answer my question either, but that's not really the point. Do you mean I should write two example, respectively? I meant an example or two of actual concrete uses (ideally the most common ones) of the rdm parameter and what they mean in practical terms. What about

Re: [Xen-devel] [v4][PATCH 03/19] xen/vtd: create RMRR mapping

2015-06-30 Thread Chen, Tiejun
It's a matter of taste to some degree. Unless patches are really involved, I prefer them not to add dead code. Apart from eliminating the case of the code remaining dead (perhaps for extended periods of time) if only parts of a series get applied, it also generally helps review if one can see the

Re: [Xen-devel] [v4][PATCH 14/19] tools/libxl: detect and avoid conflicts with RDM

2015-06-29 Thread Chen, Tiejun
static int libxl__xc_device_get_rdm(libxl__gc *gc, uint32_t flag, uint16_t seg, uint8_t bus, uint8_t devfn, unsigned int *nr_entries, struct

Re: [Xen-devel] [v4][PATCH 11/19] tools: introduce some new parameters to set rdm policy

2015-06-29 Thread Chen, Tiejun
Having read all these docs I now know what all the options are, but I still don't really know what I should write. I think an example or two of real world usage would be helpful. Here I picked some code fragments to help you understand this, I meant an example or two in the documentation.

Re: [Xen-devel] [v4][PATCH 11/19] tools: introduce some new parameters to set rdm policy

2015-06-29 Thread Chen, Tiejun
On 2015/6/25 20:31, Ian Jackson wrote: Tiejun Chen writes ([v4][PATCH 11/19] tools: introduce some new parameters to set rdm policy): This patch introduces user configurable parameters to specify RDM resource and according policies, ... Global RDM parameter, type, allows user to specify

Re: [Xen-devel] [v4][PATCH 12/19] tools/libxl: passes rdm reservation policy

2015-06-29 Thread Chen, Tiejun
+Brdm policy is about how to handle conflict between reserving reserved device +memory and guest address space. strict means an unsolved conflict leads to +immediate VM crash, while relaxed allows VM moving forward with a warning +message thrown out. Here strict is default. Surely it would be

Re: [Xen-devel] [v4][PATCH 11/19] tools: introduce some new parameters to set rdm policy

2015-06-28 Thread Chen, Tiejun
Just let me rephrase this, none means we don't check any reserved regions and then all rdm policies would be ignored, so guest just work as before. When or why would I write: rdm = none in my configuration file instead of just not saying anything? As you know we just have two

Re: [Xen-devel] [v4][PATCH 15/19] tools: introduce a new parameter to set a predefined rdm boundary

2015-06-26 Thread Chen, Tiejun
diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 index 638b350..079465f 100644 --- a/docs/man/xl.cfg.pod.5 +++ b/docs/man/xl.cfg.pod.5 @@ -767,6 +767,27 @@ to a given device, and strict is default here. Note this would override global Brdm option. +=item Brdm_mem_boundary=MBYTES +

Re: [Xen-devel] [v4][PATCH 12/19] tools/libxl: passes rdm reservation policy

2015-06-26 Thread Chen, Tiejun
BBDF is the PCI Bus/Device/Function of the physical device to pass-through. +Brdm policy is about how to handle conflict between reserving reserved device s/is about/specifies/ Okay. and I think s/between/while/ +memory and guest address space. strict means an unsolved conflict leads to

Re: [Xen-devel] [v4][PATCH 11/19] tools: introduce some new parameters to set rdm policy

2015-06-26 Thread Chen, Tiejun
Thanks for your all corrections. +=item Btype=STRING + +Currently we just have two types: Currently there are only two types. Although I would probably just say Valid types are So let say Currently there are only two valid types. +host means all reserved device memory on this platform

Re: [Xen-devel] [v4][PATCH 11/19] tools: introduce some new parameters to set rdm policy

2015-06-26 Thread Chen, Tiejun
TODO: in the future this parameter may be further extended to allow specifying arbitrary regions, e.g. even those belonging to another platform as a preparation for live migration with passthrough devices. I don't think this needs to be explained in this document at all. Whenever someone does

Re: [Xen-devel] [v4][PATCH 12/19] tools/libxl: passes rdm reservation policy

2015-06-26 Thread Chen, Tiejun
I will delete this last sentence and add this, Please refer reserve option to the rdm option in xl.cfg.5.txt. Say xl.cfg(5) which is a neutral reference to the manpage in any form. I'm trying to follow an existing example here, so This is same as reserve option to the rdm option, please

Re: [Xen-devel] [v4][PATCH 14/19] tools/libxl: detect and avoid conflicts with RDM

2015-06-25 Thread Chen, Tiejun
On 2015/6/25 19:23, Wei Liu wrote: On Tue, Jun 23, 2015 at 05:57:25PM +0800, Tiejun Chen wrote: While building a VM, HVM domain builder provides struct hvm_info_table{} to help hvmloader. Currently it includes two fields to construct guest e820 table by hvmloader, low_mem_pgend and

Re: [Xen-devel] [v4][PATCH 13/19] tools/libxc: check to set args.mmio_size before call xc_hvm_build

2015-06-25 Thread Chen, Tiejun
On 2015/6/25 19:08, Wei Liu wrote: On Tue, Jun 23, 2015 at 05:57:24PM +0800, Tiejun Chen wrote: After commit 5dff8e9eedc7, libxc/libxl: fill xc_hvm_build_args in libxl is introduced, we won't check to set args.mmio_size inside xc_hvm_build as before. So instead, we need to do this before call

Re: [Xen-devel] [v4][PATCH 03/19] xen/vtd: create RMRR mapping

2015-06-24 Thread Chen, Tiejun
Note actually we just need p2m_remove_page() to unmap these mapping on both ept and vt-d sides, and guest_physmap_remove_page is really a wrapper of p2m_remove_page(). And I agree with Tim regarding the desire to avoid code duplication. Yet that's no reason to do it asymmetrically:

Re: [Xen-devel] [v3][PATCH 07/16] hvmloader/pci: skip reserved ranges

2015-06-23 Thread Chen, Tiejun
On 2015/6/19 10:02, Chen, Tiejun wrote: On 2015/6/18 16:05, Jan Beulich wrote: On 18.06.15 at 09:01, tiejun.c...@intel.com wrote: On 2015/6/18 14:29, Jan Beulich wrote: On 18.06.15 at 08:17, tiejun.c...@intel.com wrote: On 2015/6/17 17:24, Jan Beulich wrote: On 17.06.15 at 11:18, tiejun.c

Re: [Xen-devel] [v4][PATCH 03/19] xen/vtd: create RMRR mapping

2015-06-23 Thread Chen, Tiejun
On 2015/6/23 18:12, Jan Beulich wrote: On 23.06.15 at 11:57, tiejun.c...@intel.com wrote: --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -1839,7 +1839,7 @@ static int rmrr_identity_mapping(struct domain *d, bool_t map, while ( base_pfn

Re: [Xen-devel] [v3][PATCH 07/16] hvmloader/pci: skip reserved ranges

2015-06-18 Thread Chen, Tiejun
On 2015/6/18 16:05, Jan Beulich wrote: On 18.06.15 at 09:01, tiejun.c...@intel.com wrote: On 2015/6/18 14:29, Jan Beulich wrote: On 18.06.15 at 08:17, tiejun.c...@intel.com wrote: On 2015/6/17 17:24, Jan Beulich wrote: On 17.06.15 at 11:18, tiejun.c...@intel.com wrote: On 2015/6/17 17:02,

Re: [Xen-devel] [v3][PATCH 03/16] xen/vtd: create RMRR mapping

2015-06-18 Thread Chen, Tiejun
On 2015/6/18 18:07, Tim Deegan wrote: At 14:13 +0800 on 12 Jun (1434118407), Chen, Tiejun wrote: could you explain why existing guest_physmap_remove_page can't serve the purpose so you need invent a new identity mapping specific one? For unmapping suppose it should be common regardless

Re: [Xen-devel] [v3][PATCH 07/16] hvmloader/pci: skip reserved ranges

2015-06-18 Thread Chen, Tiejun
On 2015/6/18 14:29, Jan Beulich wrote: On 18.06.15 at 08:17, tiejun.c...@intel.com wrote: On 2015/6/17 17:24, Jan Beulich wrote: On 17.06.15 at 11:18, tiejun.c...@intel.com wrote: On 2015/6/17 17:02, Jan Beulich wrote: On 17.06.15 at 10:26, tiejun.c...@intel.com wrote: Something hits me to

Re: [Xen-devel] [v3][PATCH 04/16] xen/passthrough: extend hypercall to support rdm reservation policy

2015-06-18 Thread Chen, Tiejun
On 2015/6/17 18:11, Jan Beulich wrote: On 11.06.15 at 03:15, tiejun.c...@intel.com wrote: @@ -899,7 +899,7 @@ int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn, } int set_identity_p2m_entry(struct domain *d, unsigned long gfn, - p2m_access_t

Re: [Xen-devel] [v3][PATCH 03/16] xen/vtd: create RMRR mapping

2015-06-18 Thread Chen, Tiejun
On 2015/6/17 18:03, Jan Beulich wrote: On 11.06.15 at 03:15, tiejun.c...@intel.com wrote: --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -927,10 +927,16 @@ int set_identity_p2m_entry(struct domain *d, unsigned long gfn, } gfn_unlock(p2m, gfn, 0); -

Re: [Xen-devel] [v3][PATCH 04/16] xen/passthrough: extend hypercall to support rdm reservation policy

2015-06-18 Thread Chen, Tiejun
On 2015/6/18 15:53, Jan Beulich wrote: On 18.06.15 at 09:14, tiejun.c...@intel.com wrote: On 2015/6/17 18:11, Jan Beulich wrote: On 11.06.15 at 03:15, tiejun.c...@intel.com wrote: @@ -1577,9 +1578,10 @@ int iommu_do_pci_domctl( seg = machine_sbdf 16; bus =

Re: [Xen-devel] [v3][PATCH 05/16] xen: enable XENMEM_memory_map in hvm

2015-06-18 Thread Chen, Tiejun
On 2015/6/17 18:14, Jan Beulich wrote: On 11.06.15 at 03:15, tiejun.c...@intel.com wrote: This patch enables XENMEM_memory_map in hvm. So we can use it to setup the e820 mappings. I think saying hvmloader instead of we would make things more explicit. In the context here, we would be the

Re: [Xen-devel] [v3][PATCH 16/16] xen/vtd: prevent from assign the device with shared rmrr

2015-06-18 Thread Chen, Tiejun
On 2015/6/17 18:28, Jan Beulich wrote: On 11.06.15 at 03:15, tiejun.c...@intel.com wrote: --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -2277,13 +2277,37 @@ static int intel_iommu_assign_device( if ( list_empty(acpi_drhd_units) ) return

Re: [Xen-devel] [v3][PATCH 07/16] hvmloader/pci: skip reserved ranges

2015-06-18 Thread Chen, Tiejun
On 2015/6/17 17:24, Jan Beulich wrote: On 17.06.15 at 11:18, tiejun.c...@intel.com wrote: On 2015/6/17 17:02, Jan Beulich wrote: On 17.06.15 at 10:26, tiejun.c...@intel.com wrote: Something hits me to generate another idea, #1. Still allocate all devices as before. #2. Lookup all actual bars

Re: [Xen-devel] [v3][PATCH 07/16] hvmloader/pci: skip reserved ranges

2015-06-17 Thread Chen, Tiejun
On 2015/6/17 16:05, Jan Beulich wrote: On 17.06.15 at 09:54, tiejun.c...@intel.com wrote: On 2015/6/17 15:19, Jan Beulich wrote: On 17.06.15 at 09:10, tiejun.c...@intel.com wrote: Yeah, this may waste some spaces in this worst case but I this think this can guarantee our change don't impact

Re: [Xen-devel] [v3][PATCH 07/16] hvmloader/pci: skip reserved ranges

2015-06-17 Thread Chen, Tiejun
On 2015/6/16 17:40, Jan Beulich wrote: On 16.06.15 at 11:29, tiejun.c...@intel.com wrote: I'm trying to walk into this direction: /* * We'll skip all space overlapping with reserved memory later, * so we need to increase mmio_total to compensate them. */ for (

Re: [Xen-devel] [v3][PATCH 07/16] hvmloader/pci: skip reserved ranges

2015-06-17 Thread Chen, Tiejun
On 2015/6/17 15:19, Jan Beulich wrote: On 17.06.15 at 09:10, tiejun.c...@intel.com wrote: On 2015/6/16 17:40, Jan Beulich wrote: On 16.06.15 at 11:29, tiejun.c...@intel.com wrote: I'm trying to walk into this direction: /* * We'll skip all space overlapping with reserved

Re: [Xen-devel] [v3][PATCH 07/16] hvmloader/pci: skip reserved ranges

2015-06-17 Thread Chen, Tiejun
On 2015/6/17 16:26, Chen, Tiejun wrote: On 2015/6/17 16:05, Jan Beulich wrote: On 17.06.15 at 09:54, tiejun.c...@intel.com wrote: On 2015/6/17 15:19, Jan Beulich wrote: On 17.06.15 at 09:10, tiejun.c...@intel.com wrote: Yeah, this may waste some spaces in this worst case but I this think

Re: [Xen-devel] [v3][PATCH 07/16] hvmloader/pci: skip reserved ranges

2015-06-17 Thread Chen, Tiejun
On 2015/6/17 17:02, Jan Beulich wrote: On 17.06.15 at 10:26, tiejun.c...@intel.com wrote: Something hits me to generate another idea, #1. Still allocate all devices as before. #2. Lookup all actual bars to check if they're conflicting RMRR We can skip these bars to keep zero. Then later it

Re: [Xen-devel] [v3][PATCH 15/16] xen/vtd: enable USB device assignment

2015-06-16 Thread Chen, Tiejun
On 2015/6/16 13:58, Tian, Kevin wrote: From: Chen, Tiejun Sent: Friday, June 12, 2015 5:00 PM On 2015/6/11 18:22, Tian, Kevin wrote: From: Chen, Tiejun Sent: Thursday, June 11, 2015 9:15 AM Before we refine RMRR mechanism, USB RMRR may conflict with guest bios region so we always ignore USB

Re: [Xen-devel] [v3][PATCH 07/16] hvmloader/pci: skip reserved ranges

2015-06-16 Thread Chen, Tiejun
On 2015/6/16 13:47, Tian, Kevin wrote: From: Chen, Tiejun Sent: Friday, June 12, 2015 3:54 PM bar_data |= (uint32_t)base; bar_data_upper = (uint32_t)(base 32); +for ( j = 0; j memory_map.nr_map ; j++ ) +{ +if ( memory_map.map[j].type

<    1   2   3   4   >